[PATCH] drm/amdgpu: updated UMC error address record with correct channel index

[Clements, John]

[AMD Official Use Only - Internal Distribution Only]

Resolved issue with inputting an incorrect UMC channel index into the UMC error 
address record.

Defined macros for repetitive for loops

Thank you,
John Clements


0001-drm-amdgpu-updated-UMC-error-address-record-with-cor.patch
Description: 0001-drm-amdgpu-updated-UMC-error-address-record-with-cor.patch
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amdgpu: resolved bug in UMC RAS CE query

[Chen, Guchun]

[AMD Public Use]

Reviewed-and-tested-by: Guchun Chen 


From: Clements, John 
Sent: Tuesday, January 7, 2020 11:54 AM
To: amd-gfx@lists.freedesktop.org; dl.srdc_lnx_ras 
Subject: [PATCH] drm/amdgpu: resolved bug in UMC RAS CE query


[AMD Official Use Only - Internal Distribution Only]

Submitting patch to access CE registers via SMN and disable UMC indexing mode.

Thank you,
John Clements
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amdgpu: resolved bug in UMC RAS CE query

[Clements, John]

[AMD Official Use Only - Internal Distribution Only]

Not necessary, but I wanted to make the register all access’ consistent.

In a future patch I shall replace the MMIO register offsets with the SMN 
offsets directly instead of having *4 all over the place.

Thank you,
John Clements

From: Zhou1, Tao 
Sent: Tuesday, January 7, 2020 1:59 PM
To: Clements, John ; amd-gfx@lists.freedesktop.org; 
dl.srdc_lnx_ras 
Subject: RE: [PATCH] drm/amdgpu: resolved bug in UMC RAS CE query


[AMD Official Use Only - Internal Distribution Only]

Reviewed-by: Tao Zhou mailto:tao.zh...@amd.com>>

BTW, are you sure replacing RREG32/WREG32 with RREG32/WREG32_PCIE is also 
necessary to fix the bug?

Regards,
Tao

From: Clements, John mailto:john.cleme...@amd.com>>
Sent: 2020年1月7日 11:54
To: amd-gfx@lists.freedesktop.org; 
dl.srdc_lnx_ras mailto:dl.srdc_lnx_...@amd.com>>
Subject: [PATCH] drm/amdgpu: resolved bug in UMC RAS CE query


[AMD Official Use Only - Internal Distribution Only]

Submitting patch to access CE registers via SMN and disable UMC indexing mode.

Thank you,
John Clements
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amdgpu: resolved bug in UMC RAS CE query

[Zhou1, Tao]

[AMD Official Use Only - Internal Distribution Only]

Reviewed-by: Tao Zhou mailto:tao.zh...@amd.com>>

BTW, are you sure replacing RREG32/WREG32 with RREG32/WREG32_PCIE is also 
necessary to fix the bug?

Regards,
Tao

From: Clements, John 
Sent: 2020年1月7日 11:54
To: amd-gfx@lists.freedesktop.org; dl.srdc_lnx_ras 
Subject: [PATCH] drm/amdgpu: resolved bug in UMC RAS CE query


[AMD Official Use Only - Internal Distribution Only]

Submitting patch to access CE registers via SMN and disable UMC indexing mode.

Thank you,
John Clements
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: RX 5500 XT hangs kernel on boot

[Alex Deucher]

On Mon, Jan 6, 2020 at 11:59 PM Arvid Brodin  wrote:
>
> Hi,
>
> So I tried to use my RX 5500 XT for the first time today. Kernel updated to 
> 5.4.8, fresh navi14 firmware files fetched from the linux-firmware repo, and 
> of course power cable connected to the card.
>
> Booting the kernel hangs when the graphics are initialized (on two different 
> OS:es: Ubuntu 18.04 and Gentoo, Ubuntu with linux-5.4.7 and firmware updated 
> today, Gentoo with 5.4.8 and firmware from 2019-12-15). There's nothing on 
> the screen except sometimes a non-blinking cursor - I get no error messages.
>
> So I tried to boot with the iGPU as primary with the RX 5500 XT still in the 
> box. This works, the card is detected by the kernel, firmware loads etc, and 
> it shows up in xrandr. But when enabling the output (something like 'xrandr 
> --output DisplayPort-1-2 --preferred --same-as HDMI-1') the computer 
> "half-freezes", as in, I get a little bit of reaction every 10 seconds or so 
> - caps lock change, a kernel log line is printed, etc. Mouse cursor moves but 
> system does not react to clicks.
>
> Kernel logs at this time looks like this (copied by hand from photo):
>
> kernel: [drm:amdgpu_dm_commit_planes.constprop.0 [amdgpu]] *ERROR* Waiting 
> for fences timed out!
> kernel: Asynchronous wait on fence drm_sched:gfx_0.0.0:11 timed out 
> (hint:submit_notify+0x0/0x80 [i915])
> kernel: [drm:amdgpu_dm_commit_planes.constprop.0 [amdgpu]] *ERROR* Waiting 
> for fences timed out!
> kernel: Asynchronous wait on fence drm_sched:gfx_0.0.0:12 timed out 
> (hint:submit_notify+0x0/0x80 [i915])
>
> etc... (a new pair of lines appear about every 11 seconds).
>
> I also get noise/nonsense output on the RX 5500 XT's DisplayPort at this time.
>
>
> I'd like to get my graphics card working... :-/ Any ideas?

Please try a 5.5 kernel.  Some users are reporting problems on 5.4.

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

RX 5500 XT hangs kernel on boot

[Arvid Brodin]

Hi,

So I tried to use my RX 5500 XT for the first time today. Kernel updated to 
5.4.8, fresh navi14 firmware files fetched from the linux-firmware repo, and of 
course power cable connected to the card.

Booting the kernel hangs when the graphics are initialized (on two different 
OS:es: Ubuntu 18.04 and Gentoo, Ubuntu with linux-5.4.7 and firmware updated 
today, Gentoo with 5.4.8 and firmware from 2019-12-15). There's nothing on the 
screen except sometimes a non-blinking cursor - I get no error messages.

So I tried to boot with the iGPU as primary with the RX 5500 XT still in the box. This 
works, the card is detected by the kernel, firmware loads etc, and it shows up in xrandr. 
But when enabling the output (something like 'xrandr --output DisplayPort-1-2 --preferred 
--same-as HDMI-1') the computer "half-freezes", as in, I get a little bit of 
reaction every 10 seconds or so - caps lock change, a kernel log line is printed, etc. 
Mouse cursor moves but system does not react to clicks.

Kernel logs at this time looks like this (copied by hand from photo):

kernel: [drm:amdgpu_dm_commit_planes.constprop.0 [amdgpu]] *ERROR* Waiting for 
fences timed out!
kernel: Asynchronous wait on fence drm_sched:gfx_0.0.0:11 timed out 
(hint:submit_notify+0x0/0x80 [i915])
kernel: [drm:amdgpu_dm_commit_planes.constprop.0 [amdgpu]] *ERROR* Waiting for 
fences timed out!
kernel: Asynchronous wait on fence drm_sched:gfx_0.0.0:12 timed out 
(hint:submit_notify+0x0/0x80 [i915])

etc... (a new pair of lines appear about every 11 seconds).

I also get noise/nonsense output on the RX 5500 XT's DisplayPort at this time.


I'd like to get my graphics card working... :-/ Any ideas?


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

[PATCH] drm/amdgpu: resolved bug in UMC RAS CE query

[Clements, John]

[AMD Official Use Only - Internal Distribution Only]

Submitting patch to access CE registers via SMN and disable UMC indexing mode.

Thank you,
John Clements


0001-drm-amdgpu-resolved-bug-in-UMC-RAS-CE-query.patch
Description: 0001-drm-amdgpu-resolved-bug-in-UMC-RAS-CE-query.patch
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: Problem whit Radeon rx 590 makes games crash on Linux

[Liu, Zhan]

Hi there,

Thank you for raising this question. Here are my two cents that came from my 
own experience:

>From what you mentioned in the community thread, you tried multiple kernel 
>versions on vanilla Manjaro. However, it seems like you didn't upgrade any 
>user-mode driver, and I suspect that's why you encountered so many problems 
>when playing games. If that's the case, you can try install the official AMD 
>RX590 Linux driver: 
>https://www.amd.com/en/support/graphics/radeon-500-series/radeon-rx-500-series/radeon-rx-590.
> I would recommend you use the following installation command: 
>./amdgpu-pro-install

Another possibility is, your graphics card is experiencing heat throttling 
under heavy load, and that's also a common reason why graphics cards are 
struggling a lot on heavy duty games. If that's the case, you can try adding 
more fans to your desktop.

Also, are you using PCI-e x8 on your motherboard? If that's the case, please 
switch to PCI-e x16 to solve the bottleneck.

You could also switch to another Linux distribution (e.g. Ubuntu) and give it a 
try, though I will be a bit surprised if the issue was caused by specific Linux 
distribution.

Hope that helps. Should you have more questions, please feel free to contact us 
on the mailing list.

Thanks,
Zhan

> -Original Message-
> From: amd-gfx  On Behalf Of
> Martin Bångens
> Sent: 2020/January/06, Monday 2:34 PM
> To: amd-gfx@lists.freedesktop.org
> Subject: Problem whit Radeon rx 590 makes games crash on Linux
> 
> Hi AMD graphic driver developer for Linux!
> 
> I have problems with playing games using Linux opensource driver
> 
> I have xfx radeon rx 590 fatboy and tested with proprietary driver games run
> fine but too slow for playing
> 
> here is bit more info about my experience
> 
> ask me anything
> 
> 
> https://community.amd.com/thread/246894 
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org

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

RE: [PATCH 2/5] drm/amdgpu: export function to flush TLB via pasid

[Sierra Guiza, Alejandro (Alex)]

[AMD Official Use Only - Internal Distribution Only]



-Original Message-
From: Koenig, Christian  
Sent: Monday, January 6, 2020 10:23 AM
To: Kuehling, Felix ; amd-gfx@lists.freedesktop.org; 
Sierra Guiza, Alejandro (Alex) 
Subject: Re: [PATCH 2/5] drm/amdgpu: export function to flush TLB via pasid

Am 06.01.20 um 17:04 schrieb Felix Kuehling:
> On 2020-01-05 10:41 a.m., Christian König wrote:
>> Am 20.12.19 um 07:24 schrieb Alex Sierra:
>>> This can be used directly from amdgpu and amdkfd to invalidate TLB 
>>> through pasid.
>>> It supports gmc v7, v8, v9 and v10.
>>>
>>> Change-Id: I6563a8eba2e42d1a67fa2547156c20da41d1e490
>>> Signed-off-by: Alex Sierra 
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  6 ++
>>>   drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 81 
>>> 
>>>   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c   | 33 ++
>>>   drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   | 34 ++
>>>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 84
>>> +
>>>   5 files changed, 238 insertions(+)
> [snip]
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> index fa025ceeea0f..eb1e64bd56ed 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> @@ -38,10 +38,12 @@
>>>   #include "dce/dce_12_0_sh_mask.h"
>>>   #include "vega10_enum.h"
>>>   #include "mmhub/mmhub_1_0_offset.h"
>>> +#include "athub/athub_1_0_sh_mask.h"
>>>   #include "athub/athub_1_0_offset.h"
>>>   #include "oss/osssys_4_0_offset.h"
>>>     #include "soc15.h"
>>> +#include "soc15d.h"
>>>   #include "soc15_common.h"
>>>   #include "umc/umc_6_0_sh_mask.h"
>>>   @@ -434,6 +436,47 @@ static bool
>>> gmc_v9_0_use_invalidate_semaphore(struct amdgpu_device *adev,
>>>  adev->pdev->device == 0x15d8)));
>>>   }
>>>   +static bool gmc_v9_0_get_atc_vmid_pasid_mapping_info(struct
>>> amdgpu_device *adev,
>>> +    uint8_t vmid, uint16_t *p_pasid) {
>>> +    uint32_t value;
>>> +
>>> +    value = RREG32(SOC15_REG_OFFSET(ATHUB, 0,
>>> mmATC_VMID0_PASID_MAPPING)
>>> + + vmid);
>>> +    *p_pasid = value & ATC_VMID0_PASID_MAPPING__PASID_MASK;
>>> +
>>> +    return !!(value & ATC_VMID0_PASID_MAPPING__VALID_MASK);
>>> +}
>>
>> Probably better to expose just this function in the GMC interface.
>
> This is called below in gmc_v9_0_flush_gpu_tlb_pasid in the case that 
> the KIQ is not ready. It is not needed outside this file, so no need 
> to expose it in the GMC interface.
>
>
>>
>>> +
>>> +static int gmc_v9_0_invalidate_tlbs_with_kiq(struct amdgpu_device
>>> *adev,
>>> +    uint16_t pasid, uint32_t flush_type,
>>> +    bool all_hub)
>>> +{
>>> +    signed long r;
>>> +    uint32_t seq;
>>> +    struct amdgpu_ring *ring = >gfx.kiq.ring;
>>> +
>>> +    spin_lock(>gfx.kiq.ring_lock);
>>> +    amdgpu_ring_alloc(ring, 12); /* fence + invalidate_tlbs 
>>> +package*/
>>> +    amdgpu_ring_write(ring, PACKET3(PACKET3_INVALIDATE_TLBS, 0));
>>> +    amdgpu_ring_write(ring,
>>> +    PACKET3_INVALIDATE_TLBS_DST_SEL(1) |
>>> +    PACKET3_INVALIDATE_TLBS_ALL_HUB(all_hub) |
>>> +    PACKET3_INVALIDATE_TLBS_PASID(pasid) |
>>> +    PACKET3_INVALIDATE_TLBS_FLUSH_TYPE(flush_type));
>>
>>> That stuff is completely unrelated to the GMC and shouldn't be added
>>> here.
>>
>> Where else should it go? To me TLB flushing is very much something 
>> that belongs in this file.
>>
>> Maybe you'd rather add "flush_tlbs_with_kiq" to amdgpu_ring_funcs and
>> implement it in the gfx_v*.c GFX-IP code? I'm not sure that makes much 
>> sense either because it's only available on the KIQ ring.

>Yes, something like this. We should probably add a amdgpu_kiq_funcs and expose 
>the functions needed by the GMC code there.

>See the amdgpu_virt_kiq_reg_write_reg_wait() case for reference, it is very 
>similar and should probably be added to that function table as well.

>Christian.
To summarize: 
1.- The idea is to add a new function pointer to the amdgpu_ring_funcs to flush 
the tlbs with kiq. 
2.- This function pointer should be pointed and implemented on each of the 
gfx_v*.c under the gfx_*_ring_funcs_kiq struct. If this is true, Im not seeing 
this struct on the gfx_v10.c file.
3.- Use the amdgpu_virt_kiq_reg_write_reg_wait as a reference for the 
implementation.

>>
>>
>>>
>>> Christian.
>>>
>>> +    amdgpu_fence_emit_polling(ring, );
>>> +    amdgpu_ring_commit(ring);
>>> +    spin_unlock(>gfx.kiq.ring_lock);
>>> +
>>> +    r = amdgpu_fence_wait_polling(ring, seq, adev->usec_timeout);
>>> +    if (r < 1) {
>>> +    DRM_ERROR("wait for kiq fence error: %ld.\n", r);
>>> +    return -ETIME;
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>>   /*
>>>    * GART
>>>    * VMID 0 is the physical GPU addresses as used by the kernel.
>>> @@ -532,6 +575,46 @@ static void gmc_v9_0_flush_gpu_tlb(struct 
>>> amdgpu_device *adev, uint32_t vmid,
>>>   

Re: [PATCH v2] drm/amd/display: Reduce HDMI pixel encoding if max clock is exceeded

[Alex Deucher]

On Thu, Jan 2, 2020 at 10:14 AM Harry Wentland  wrote:
>
> On 2019-12-02 4:47 p.m., Thomas Anderson wrote:
> > For high-res (8K) or HFR (4K120) displays, using uncompressed pixel
> > formats like YCbCr444 would exceed the bandwidth of HDMI 2.0, so the
> > "interesting" modes would be disabled, leaving only low-res or low
> > framerate modes.
> >
> > This change lowers the pixel encoding to 4:2:2 or 4:2:0 if the max TMDS
> > clock is exceeded. Verified that 8K30 and 4K120 are now available and
> > working with a Samsung Q900R over an HDMI 2.0b link from a Radeon 5700.
> >
> > Signed-off-by: Thomas Anderson 
>
> Apologies for the late response.
>
> Thanks for getting high-res modes working on HDMI.
>
> This change is
> Reviewed-by: Harry Wentland 
>

Applied.  thanks!

Alex

> Harry
>
> > ---
> >  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 45 ++-
> >  1 file changed, 23 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index 7aac9568d3be..803e59d97411 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -3356,27 +3356,21 @@ get_output_color_space(const struct dc_crtc_timing 
> > *dc_crtc_timing)
> >   return color_space;
> >  }
> >
> > -static void reduce_mode_colour_depth(struct dc_crtc_timing *timing_out)
> > -{
> > - if (timing_out->display_color_depth <= COLOR_DEPTH_888)
> > - return;
> > -
> > - timing_out->display_color_depth--;
> > -}
> > -
> > -static void adjust_colour_depth_from_display_info(struct dc_crtc_timing 
> > *timing_out,
> > - const struct drm_display_info 
> > *info)
> > +static bool adjust_colour_depth_from_display_info(
> > + struct dc_crtc_timing *timing_out,
> > + const struct drm_display_info *info)
> >  {
> > + enum dc_color_depth depth = timing_out->display_color_depth;
> >   int normalized_clk;
> > - if (timing_out->display_color_depth <= COLOR_DEPTH_888)
> > - return;
> >   do {
> >   normalized_clk = timing_out->pix_clk_100hz / 10;
> >   /* YCbCr 4:2:0 requires additional adjustment of 1/2 */
> >   if (timing_out->pixel_encoding == PIXEL_ENCODING_YCBCR420)
> >   normalized_clk /= 2;
> >   /* Adjusting pix clock following on HDMI spec based on colour 
> > depth */
> > - switch (timing_out->display_color_depth) {
> > + switch (depth) {
> > + case COLOR_DEPTH_888:
> > + break;
> >   case COLOR_DEPTH_101010:
> >   normalized_clk = (normalized_clk * 30) / 24;
> >   break;
> > @@ -3387,14 +3381,15 @@ static void 
> > adjust_colour_depth_from_display_info(struct dc_crtc_timing *timing_
> >   normalized_clk = (normalized_clk * 48) / 24;
> >   break;
> >   default:
> > - return;
> > + /* The above depths are the only ones valid for HDMI. 
> > */
> > + return false;
> >   }
> > - if (normalized_clk <= info->max_tmds_clock)
> > - return;
> > - reduce_mode_colour_depth(timing_out);
> > -
> > - } while (timing_out->display_color_depth > COLOR_DEPTH_888);
> > -
> > + if (normalized_clk <= info->max_tmds_clock) {
> > + timing_out->display_color_depth = depth;
> > + return true;
> > + }
> > + } while (--depth > COLOR_DEPTH_666);
> > + return false;
> >  }
> >
> >  static void fill_stream_properties_from_drm_display_mode(
> > @@ -3474,8 +3469,14 @@ static void 
> > fill_stream_properties_from_drm_display_mode(
> >
> >   stream->out_transfer_func->type = TF_TYPE_PREDEFINED;
> >   stream->out_transfer_func->tf = TRANSFER_FUNCTION_SRGB;
> > - if (stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
> > - adjust_colour_depth_from_display_info(timing_out, info);
> > + if (stream->signal == SIGNAL_TYPE_HDMI_TYPE_A) {
> > + if (!adjust_colour_depth_from_display_info(timing_out, info) 
> > &&
> > + drm_mode_is_420_also(info, mode_in) &&
> > + timing_out->pixel_encoding != PIXEL_ENCODING_YCBCR420) {
> > + timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR420;
> > + adjust_colour_depth_from_display_info(timing_out, 
> > info);
> > + }
> > + }
> >  }
> >
> >  static void fill_audio_info(struct audio_info *audio_info,
> >
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list

Re: [PATCH] drm/amd/powerplay: issue proper hdp flush for table transferring

[Alex Deucher]

On Mon, Jan 6, 2020 at 1:34 AM Quan, Evan  wrote:
>
> Ping..
>

Reviewed-by: Alex Deucher 

> > -Original Message-
> > From: Quan, Evan 
> > Sent: Friday, January 3, 2020 5:47 PM
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Quan, Evan 
> > Subject: [PATCH] drm/amd/powerplay: issue proper hdp flush for table
> > transferring
> >
> > Guard the content consistence between the view of GPU and CPU during the
> > table transferring.
> >
> > Change-Id: Ib3cebb97a1c8fb302eb040483bbaf089ae00c6a9
> > Signed-off-by: Evan Quan 
> > ---
> >  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c| 15 ++-
> >  .../gpu/drm/amd/powerplay/smumgr/smu10_smumgr.c   |  5 -
> >  .../gpu/drm/amd/powerplay/smumgr/vega10_smumgr.c  |  5 -
> >   .../gpu/drm/amd/powerplay/smumgr/vega12_smumgr.c  |  5 -
> >   .../gpu/drm/amd/powerplay/smumgr/vega20_smumgr.c  | 10 --
> >  5 files changed, 30 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> > b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> > index a56ebcc4e3c7..e1b64134bbd8 100644
> > --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> > +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> > @@ -529,8 +529,14 @@ int smu_update_table(struct smu_context *smu,
> > enum smu_table_id table_index, int
> >
> >   table_size = smu_table->tables[table_index].size;
> >
> > - if (drv2smu)
> > + if (drv2smu) {
> >   memcpy(table->cpu_addr, table_data, table_size);
> > + /*
> > +  * Flush hdp cache: to guard the content seen by
> > +  * GPU is consitent with CPU.
> > +  */
> > + amdgpu_asic_flush_hdp(adev, NULL);
> > + }
> >
> >   ret = smu_send_smc_msg_with_param(smu, drv2smu ?
> > SMU_MSG_TransferTableDram2Smu :
> > @@ -539,11 +545,10 @@ int smu_update_table(struct smu_context *smu,
> > enum smu_table_id table_index, int
> >   if (ret)
> >   return ret;
> >
> > - /* flush hdp cache */
> > - adev->nbio.funcs->hdp_flush(adev, NULL);
> > -
> > - if (!drv2smu)
> > + if (!drv2smu) {
> > + amdgpu_asic_flush_hdp(adev, NULL);
> >   memcpy(table_data, table->cpu_addr, table_size);
> > + }
> >
> >   return ret;
> >  }
> > diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/smu10_smumgr.c
> > b/drivers/gpu/drm/amd/powerplay/smumgr/smu10_smumgr.c
> > index aa0ee2b46135..2319400a3fcb 100644
> > --- a/drivers/gpu/drm/amd/powerplay/smumgr/smu10_smumgr.c
> > +++ b/drivers/gpu/drm/amd/powerplay/smumgr/smu10_smumgr.c
> > @@ -137,7 +137,7 @@ static int smu10_copy_table_from_smc(struct
> > pp_hwmgr *hwmgr,
> >   priv->smu_tables.entry[table_id].table_id);
> >
> >   /* flush hdp cache */
> > - adev->nbio.funcs->hdp_flush(adev, NULL);
> > + amdgpu_asic_flush_hdp(adev, NULL);
> >
> >   memcpy(table, (uint8_t *)priv->smu_tables.entry[table_id].table,
> >   priv->smu_tables.entry[table_id].size);
> > @@ -150,6 +150,7 @@ static int smu10_copy_table_to_smc(struct pp_hwmgr
> > *hwmgr,  {
> >   struct smu10_smumgr *priv =
> >   (struct smu10_smumgr *)(hwmgr->smu_backend);
> > + struct amdgpu_device *adev = hwmgr->adev;
> >
> >   PP_ASSERT_WITH_CODE(table_id < MAX_SMU_TABLE,
> >   "Invalid SMU Table ID!", return -EINVAL;); @@ -161,6
> > +162,8 @@ static int smu10_copy_table_to_smc(struct pp_hwmgr *hwmgr,
> >   memcpy(priv->smu_tables.entry[table_id].table, table,
> >   priv->smu_tables.entry[table_id].size);
> >
> > + amdgpu_asic_flush_hdp(adev, NULL);
> > +
> >   smu10_send_msg_to_smc_with_parameter(hwmgr,
> >   PPSMC_MSG_SetDriverDramAddrHigh,
> >   upper_32_bits(priv-
> > >smu_tables.entry[table_id].mc_addr));
> > diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/vega10_smumgr.c
> > b/drivers/gpu/drm/amd/powerplay/smumgr/vega10_smumgr.c
> > index 39427ca32a15..715564009089 100644
> > --- a/drivers/gpu/drm/amd/powerplay/smumgr/vega10_smumgr.c
> > +++ b/drivers/gpu/drm/amd/powerplay/smumgr/vega10_smumgr.c
> > @@ -58,7 +58,7 @@ static int vega10_copy_table_from_smc(struct pp_hwmgr
> > *hwmgr,
> >   priv->smu_tables.entry[table_id].table_id);
> >
> >   /* flush hdp cache */
> > - adev->nbio.funcs->hdp_flush(adev, NULL);
> > + amdgpu_asic_flush_hdp(adev, NULL);
> >
> >   memcpy(table, priv->smu_tables.entry[table_id].table,
> >   priv->smu_tables.entry[table_id].size);
> > @@ -70,6 +70,7 @@ static int vega10_copy_table_to_smc(struct pp_hwmgr
> > *hwmgr,
> >   uint8_t *table, int16_t table_id)
> >  {
> >   struct vega10_smumgr *priv = hwmgr->smu_backend;
> > + struct amdgpu_device *adev = hwmgr->adev;
> >
> >   /* under sriov, vbios or hypervisor driver
> >* has already copy table to smc so here 

Re: [PATCH] drm/amd/powerplay: cleanup the interfaces for powergate setting through SMU

[Alex Deucher]

On Fri, Jan 3, 2020 at 4:47 AM Evan Quan  wrote:
>
> Provided an unified entry point. And fixed the confusing that the API
> usage is conflict with what the naming implies.

At some point it would be nice to unify the interfaces between
powerplay and swSMU so we don't seem all the is_sw_smu checks, but for
now,
Reviewed-by: Alex Deucher 

>
> Change-Id: If068980ca6a7b223d0b4d087cd99cdeb729b0e77
> Signed-off-by: Evan Quan 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.c| 23 ++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 43 --
>  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c |  6 +--
>  3 files changed, 37 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.c
> index 9cc270efee7c..cd76fbf4385d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.c
> @@ -951,16 +951,31 @@ int amdgpu_dpm_set_powergating_by_smu(struct 
> amdgpu_device *adev, uint32_t block
> case AMD_IP_BLOCK_TYPE_VCN:
> case AMD_IP_BLOCK_TYPE_VCE:
> case AMD_IP_BLOCK_TYPE_SDMA:
> +   if (swsmu) {
> +   ret = smu_dpm_set_power_gate(>smu, block_type, 
> gate);
> +   } else {
> +   if (adev->powerplay.pp_funcs &&
> +   adev->powerplay.pp_funcs->set_powergating_by_smu) 
> {
> +   mutex_lock(>pm.mutex);
> +   ret = 
> ((adev)->powerplay.pp_funcs->set_powergating_by_smu(
> +   (adev)->powerplay.pp_handle, 
> block_type, gate));
> +   mutex_unlock(>pm.mutex);
> +   }
> +   }
> +   break;
> +   case AMD_IP_BLOCK_TYPE_JPEG:
> if (swsmu)
> ret = smu_dpm_set_power_gate(>smu, block_type, 
> gate);
> -   else
> -   ret = 
> ((adev)->powerplay.pp_funcs->set_powergating_by_smu(
> -   (adev)->powerplay.pp_handle, block_type, 
> gate));
> break;
> case AMD_IP_BLOCK_TYPE_GMC:
> case AMD_IP_BLOCK_TYPE_ACP:
> -   ret = ((adev)->powerplay.pp_funcs->set_powergating_by_smu(
> +   if (adev->powerplay.pp_funcs &&
> +   adev->powerplay.pp_funcs->set_powergating_by_smu) {
> +   mutex_lock(>pm.mutex);
> +   ret = 
> ((adev)->powerplay.pp_funcs->set_powergating_by_smu(
> (adev)->powerplay.pp_handle, block_type, 
> gate));
> +   mutex_unlock(>pm.mutex);
> +   }
> break;
> default:
> break;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index b32adda70bbc..285d460624c8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -2762,17 +2762,12 @@ static void 
> amdgpu_dpm_change_power_state_locked(struct amdgpu_device *adev)
>  void amdgpu_dpm_enable_uvd(struct amdgpu_device *adev, bool enable)
>  {
> int ret = 0;
> -   if (is_support_sw_smu(adev)) {
> -   ret = smu_dpm_set_power_gate(>smu, AMD_IP_BLOCK_TYPE_UVD, 
> enable);
> -   if (ret)
> -   DRM_ERROR("[SW SMU]: dpm enable uvd failed, state = %s, ret = 
> %d. \n",
> - enable ? "true" : "false", ret);
> -   } else if (adev->powerplay.pp_funcs->set_powergating_by_smu) {
> -   /* enable/disable UVD */
> -   mutex_lock(>pm.mutex);
> -   amdgpu_dpm_set_powergating_by_smu(adev, 
> AMD_IP_BLOCK_TYPE_UVD, !enable);
> -   mutex_unlock(>pm.mutex);
> -   }
> +
> +   ret = amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_UVD, 
> !enable);
> +   if (ret)
> +   DRM_ERROR("Dpm %s uvd failed, ret = %d. \n",
> + enable ? "enable" : "disable", ret);
> +
> /* enable/disable Low Memory PState for UVD (4k videos) */
> if (adev->asic_type == CHIP_STONEY &&
> adev->uvd.decode_image_width >= WIDTH_4K) {
> @@ -2789,17 +2784,11 @@ void amdgpu_dpm_enable_uvd(struct amdgpu_device 
> *adev, bool enable)
>  void amdgpu_dpm_enable_vce(struct amdgpu_device *adev, bool enable)
>  {
> int ret = 0;
> -   if (is_support_sw_smu(adev)) {
> -   ret = smu_dpm_set_power_gate(>smu, AMD_IP_BLOCK_TYPE_VCE, 
> enable);
> -   if (ret)
> -   DRM_ERROR("[SW SMU]: dpm enable vce failed, state = %s, ret = 
> %d. \n",
> - enable ? "true" : "false", ret);
> -   } else if (adev->powerplay.pp_funcs->set_powergating_by_smu) {
> -   /* enable/disable VCE */
> -   mutex_lock(>pm.mutex);
> -   

Re: [PATCH] drm/amd: use list_for_each_entry for list iteration.

[Alex Deucher]

On Fri, Jan 3, 2020 at 2:34 PM Wambui Karuga  wrote:
>
> list_for_each() can be replaced by the more concise
> list_for_each_entry() here for iteration over the lists.
> This change was reported by coccinelle.
>
> Signed-off-by: Wambui Karuga 

Applied.  Thanks!

Alex

> ---
>  .../drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c | 19 ---
>  1 file changed, 4 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c
> index 64445c4cc4c2..cbcf504f73a5 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c
> @@ -111,17 +111,12 @@ static void init_handler_common_data(struct 
> amdgpu_dm_irq_handler_data *hcd,
>   */
>  static void dm_irq_work_func(struct work_struct *work)
>  {
> -   struct list_head *entry;
> struct irq_list_head *irq_list_head =
> container_of(work, struct irq_list_head, work);
> struct list_head *handler_list = _list_head->head;
> struct amdgpu_dm_irq_handler_data *handler_data;
>
> -   list_for_each(entry, handler_list) {
> -   handler_data = list_entry(entry,
> - struct amdgpu_dm_irq_handler_data,
> - list);
> -
> +   list_for_each_entry(handler_data, handler_list, list) {
> DRM_DEBUG_KMS("DM_IRQ: work_func: for dal_src=%d\n",
> handler_data->irq_source);
>
> @@ -528,19 +523,13 @@ static void amdgpu_dm_irq_immediate_work(struct 
> amdgpu_device *adev,
>  enum dc_irq_source irq_source)
>  {
> struct amdgpu_dm_irq_handler_data *handler_data;
> -   struct list_head *entry;
> unsigned long irq_table_flags;
>
> DM_IRQ_TABLE_LOCK(adev, irq_table_flags);
>
> -   list_for_each(
> -   entry,
> -   >dm.irq_handler_list_high_tab[irq_source]) {
> -
> -   handler_data = list_entry(entry,
> - struct amdgpu_dm_irq_handler_data,
> - list);
> -
> +   list_for_each_entry(handler_data,
> +   >dm.irq_handler_list_high_tab[irq_source],
> +   list) {
> /* Call a subcomponent which registered for immediate
>  * interrupt notification */
> handler_data->handler(handler_data->handler_arg);
> --
> 2.17.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

Re: [PATCH] drm/radeon: remove unnecessary braces around conditionals.

[Alex Deucher]

On Fri, Jan 3, 2020 at 2:34 PM Wambui Karuga  wrote:
>
> As single statement conditionals do not need to be wrapped around
> braces, the unnecessary braces can be removed.
>
> Signed-off-by: Wambui Karuga 

Applied. thanks!

Alex

> ---
>  drivers/gpu/drm/radeon/atombios_crtc.c |  3 +--
>  drivers/gpu/drm/radeon/atombios_dp.c   |  3 +--
>  drivers/gpu/drm/radeon/atombios_encoders.c |  9 -
>  drivers/gpu/drm/radeon/radeon_connectors.c |  4 ++--
>  drivers/gpu/drm/radeon/radeon_vce.c|  4 ++--
>  drivers/gpu/drm/radeon/radeon_vm.c | 16 
>  6 files changed, 18 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c 
> b/drivers/gpu/drm/radeon/atombios_crtc.c
> index da2c9e295408..be583695427a 100644
> --- a/drivers/gpu/drm/radeon/atombios_crtc.c
> +++ b/drivers/gpu/drm/radeon/atombios_crtc.c
> @@ -244,9 +244,8 @@ static void atombios_blank_crtc(struct drm_crtc *crtc, 
> int state)
>
> atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t 
> *));
>
> -   if (ASIC_IS_DCE8(rdev)) {
> +   if (ASIC_IS_DCE8(rdev))
> WREG32(vga_control_regs[radeon_crtc->crtc_id], vga_control);
> -   }
>  }
>
>  static void atombios_powergate_crtc(struct drm_crtc *crtc, int state)
> diff --git a/drivers/gpu/drm/radeon/atombios_dp.c 
> b/drivers/gpu/drm/radeon/atombios_dp.c
> index 6f38375c77c8..39b342b5c495 100644
> --- a/drivers/gpu/drm/radeon/atombios_dp.c
> +++ b/drivers/gpu/drm/radeon/atombios_dp.c
> @@ -816,9 +816,8 @@ void radeon_dp_link_train(struct drm_encoder *encoder,
> dp_info.use_dpencoder = true;
> index = GetIndexIntoMasterTable(COMMAND, DPEncoderService);
> if (atom_parse_cmd_header(rdev->mode_info.atom_context, index, , 
> )) {
> -   if (crev > 1) {
> +   if (crev > 1)
> dp_info.use_dpencoder = false;
> -   }
> }
>
> dp_info.enc_id = 0;
> diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c 
> b/drivers/gpu/drm/radeon/atombios_encoders.c
> index 2a7be5d5e7e6..cc5ee1b3af84 100644
> --- a/drivers/gpu/drm/radeon/atombios_encoders.c
> +++ b/drivers/gpu/drm/radeon/atombios_encoders.c
> @@ -1885,11 +1885,10 @@ atombios_set_encoder_crtc_source(struct drm_encoder 
> *encoder)
> if (ASIC_IS_AVIVO(rdev))
> args.v1.ucCRTC = radeon_crtc->crtc_id;
> else {
> -   if (radeon_encoder->encoder_id == 
> ENCODER_OBJECT_ID_INTERNAL_DAC1) {
> +   if (radeon_encoder->encoder_id == 
> ENCODER_OBJECT_ID_INTERNAL_DAC1)
> args.v1.ucCRTC = radeon_crtc->crtc_id;
> -   } else {
> +   else
> args.v1.ucCRTC = radeon_crtc->crtc_id 
> << 2;
> -   }
> }
> switch (radeon_encoder->encoder_id) {
> case ENCODER_OBJECT_ID_INTERNAL_TMDS1:
> @@ -2234,9 +2233,9 @@ int radeon_atom_pick_dig_encoder(struct drm_encoder 
> *encoder, int fe_idx)
> DRM_ERROR("Got encoder index incorrect - returning 0\n");
> return 0;
> }
> -   if (rdev->mode_info.active_encoders & (1 << enc_idx)) {
> +   if (rdev->mode_info.active_encoders & (1 << enc_idx))
> DRM_ERROR("chosen encoder in use %d\n", enc_idx);
> -   }
> +
> rdev->mode_info.active_encoders |= (1 << enc_idx);
> return enc_idx;
>  }
> diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c 
> b/drivers/gpu/drm/radeon/radeon_connectors.c
> index 90d2f732affb..fe12d9d91d7a 100644
> --- a/drivers/gpu/drm/radeon/radeon_connectors.c
> +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
> @@ -700,9 +700,9 @@ static int radeon_connector_set_property(struct 
> drm_connector *connector, struct
> else
> ret = 
> radeon_legacy_get_tmds_info_from_combios(radeon_encoder, tmds);
> }
> -   if (val == 1 || !ret) {
> +   if (val == 1 || !ret)
> 
> radeon_legacy_get_tmds_info_from_table(radeon_encoder, tmds);
> -   }
> +
> radeon_property_change_mode(_encoder->base);
> }
>
> diff --git a/drivers/gpu/drm/radeon/radeon_vce.c 
> b/drivers/gpu/drm/radeon/radeon_vce.c
> index 59db54ace428..5e8006444704 100644
> --- a/drivers/gpu/drm/radeon/radeon_vce.c
> +++ b/drivers/gpu/drm/radeon/radeon_vce.c
> @@ -388,9 +388,9 @@ int radeon_vce_get_create_msg(struct radeon_device *rdev, 
> int ring,
> ib.ptr[i] = cpu_to_le32(0x0);
>
> r = radeon_ib_schedule(rdev, , NULL, false);
> -   if (r) {
> +   if (r)
> DRM_ERROR("radeon: failed to schedule ib (%d).\n", r);
> -   }
> +
>
>  

Re: [PATCH] drm/radeon: remove boolean checks in if statements.

[Alex Deucher]

On Fri, Jan 3, 2020 at 2:34 PM Wambui Karuga  wrote:
>
> Remove unnecessary variable comparisions to true/false in if statements
> and check the value of the variable directly.
>
> Signed-off-by: Wambui Karuga 

Applied.  Thanks!

Alex

> ---
>  drivers/gpu/drm/radeon/cik_sdma.c   |  2 +-
>  drivers/gpu/drm/radeon/r100.c   |  2 +-
>  drivers/gpu/drm/radeon/r600.c   |  2 +-
>  drivers/gpu/drm/radeon/radeon_bios.c| 12 ++--
>  drivers/gpu/drm/radeon/radeon_connectors.c  |  4 ++--
>  drivers/gpu/drm/radeon/radeon_display.c |  4 ++--
>  drivers/gpu/drm/radeon/radeon_legacy_encoders.c |  4 ++--
>  drivers/gpu/drm/radeon/radeon_pm.c  |  2 +-
>  8 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/cik_sdma.c 
> b/drivers/gpu/drm/radeon/cik_sdma.c
> index 35b9dc6ce46a..68403e77756d 100644
> --- a/drivers/gpu/drm/radeon/cik_sdma.c
> +++ b/drivers/gpu/drm/radeon/cik_sdma.c
> @@ -333,7 +333,7 @@ void cik_sdma_enable(struct radeon_device *rdev, bool 
> enable)
> u32 me_cntl, reg_offset;
> int i;
>
> -   if (enable == false) {
> +   if (!enable) {
> cik_sdma_gfx_stop(rdev);
> cik_sdma_rlc_stop(rdev);
> }
> diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
> index 0017aa7a9f17..957994258860 100644
> --- a/drivers/gpu/drm/radeon/r100.c
> +++ b/drivers/gpu/drm/radeon/r100.c
> @@ -2815,7 +2815,7 @@ void r100_vga_set_state(struct radeon_device *rdev, 
> bool state)
> uint32_t temp;
>
> temp = RREG32(RADEON_CONFIG_CNTL);
> -   if (state == false) {
> +   if (!state) {
> temp &= ~RADEON_CFG_VGA_RAM_EN;
> temp |= RADEON_CFG_VGA_IO_DIS;
> } else {
> diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
> index 0d453aa09352..eb56fb48a6b7 100644
> --- a/drivers/gpu/drm/radeon/r600.c
> +++ b/drivers/gpu/drm/radeon/r600.c
> @@ -3191,7 +3191,7 @@ void r600_vga_set_state(struct radeon_device *rdev, 
> bool state)
> uint32_t temp;
>
> temp = RREG32(CONFIG_CNTL);
> -   if (state == false) {
> +   if (!state) {
> temp &= ~(1<<0);
> temp |= (1<<1);
> } else {
> diff --git a/drivers/gpu/drm/radeon/radeon_bios.c 
> b/drivers/gpu/drm/radeon/radeon_bios.c
> index c84d965c283e..c42f73fad3e3 100644
> --- a/drivers/gpu/drm/radeon/radeon_bios.c
> +++ b/drivers/gpu/drm/radeon/radeon_bios.c
> @@ -664,17 +664,17 @@ bool radeon_get_bios(struct radeon_device *rdev)
> uint16_t tmp;
>
> r = radeon_atrm_get_bios(rdev);
> -   if (r == false)
> +   if (!r)
> r = radeon_acpi_vfct_bios(rdev);
> -   if (r == false)
> +   if (!r)
> r = igp_read_bios_from_vram(rdev);
> -   if (r == false)
> +   if (!r)
> r = radeon_read_bios(rdev);
> -   if (r == false)
> +   if (!r)
> r = radeon_read_disabled_bios(rdev);
> -   if (r == false)
> +   if (!r)
> r = radeon_read_platform_bios(rdev);
> -   if (r == false || rdev->bios == NULL) {
> +   if (!r || rdev->bios == NULL) {
> DRM_ERROR("Unable to locate a BIOS ROM\n");
> rdev->bios = NULL;
> return false;
> diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c 
> b/drivers/gpu/drm/radeon/radeon_connectors.c
> index 0851e6817e57..90d2f732affb 100644
> --- a/drivers/gpu/drm/radeon/radeon_connectors.c
> +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
> @@ -440,7 +440,7 @@ radeon_connector_analog_encoder_conflict_solve(struct 
> drm_connector *connector,
> if (radeon_conflict->use_digital)
> continue;
>
> -   if (priority == true) {
> +   if (priority) {
> DRM_DEBUG_KMS("1: conflicting 
> encoders switching off %s\n",
>   conflict->name);
> DRM_DEBUG_KMS("in favor of %s\n",
> @@ -700,7 +700,7 @@ static int radeon_connector_set_property(struct 
> drm_connector *connector, struct
> else
> ret = 
> radeon_legacy_get_tmds_info_from_combios(radeon_encoder, tmds);
> }
> -   if (val == 1 || ret == false) {
> +   if (val == 1 || !ret) {
> 
> radeon_legacy_get_tmds_info_from_table(radeon_encoder, tmds);
> }
> radeon_property_change_mode(_encoder->base);
> diff --git a/drivers/gpu/drm/radeon/radeon_display.c 
> b/drivers/gpu/drm/radeon/radeon_display.c
> index dfb921899853..e42d9e0a4b8c 100644
> --- a/drivers/gpu/drm/radeon/radeon_display.c
> +++ 

Re: [PATCH] Revert "drm/amdgpu: Set no-retry as default."

[Felix Kuehling]

On 2020-01-06 3:45 p.m., Alex Deucher wrote:

This reverts commit 51bfac71cade386966791a8db87a5912781d249f.

This causes stability issues on some raven boards.  Revert
for now until a proper fix is completed.

Bug: https://gitlab.freedesktop.org/drm/amd/issues/934
Bug: https://bugzilla.kernel.org/show_bug.cgi?id=206017
Signed-off-by: Alex Deucher 


Reviewed-by: Felix Kuehling 



---
  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 0ffc9447b573..3036ec883fb4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -142,7 +142,7 @@ int amdgpu_async_gfx_ring = 1;
  int amdgpu_mcbp = 0;
  int amdgpu_discovery = -1;
  int amdgpu_mes = 0;
-int amdgpu_noretry = 1;
+int amdgpu_noretry;
  int amdgpu_force_asic_type = -1;
  
  struct amdgpu_mgpu_info mgpu_info = {

@@ -588,7 +588,7 @@ MODULE_PARM_DESC(mes,
  module_param_named(mes, amdgpu_mes, int, 0444);
  
  MODULE_PARM_DESC(noretry,

-   "Disable retry faults (0 = retry enabled, 1 = retry disabled 
(default))");
+   "Disable retry faults (0 = retry enabled (default), 1 = retry 
disabled)");
  module_param_named(noretry, amdgpu_noretry, int, 0644);
  
  /**

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

Re: [PATCH 08/12] drm/amdgpu: rework synchronization of VM updates

[Felix Kuehling]

On 2020-01-06 10:03 a.m., Christian König wrote:

If provided we only sync to the BOs reservation
object and no longer to the root PD.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c|  7 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 34 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  4 +--
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c  | 25 ++-
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 15 +++--
  5 files changed, 35 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
index c124f64e7aae..5816df9f8531 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
@@ -240,13 +240,6 @@ int amdgpu_sync_resv(struct amdgpu_device *adev, struct 
amdgpu_sync *sync,
owner != AMDGPU_FENCE_OWNER_UNDEFINED)
continue;
  
-		/* VM updates only sync with moves but not with user

-* command submissions or KFD evictions fences
-*/
-   if (fence_owner != AMDGPU_FENCE_OWNER_UNDEFINED &&
-   owner == AMDGPU_FENCE_OWNER_VM)
-   continue;
-
/* Ignore fences depending on the sync mode */
switch (mode) {
case AMDGPU_SYNC_ALWAYS:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 54430c9f7a27..a03cfbe670c4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -777,7 +777,7 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
params.vm = vm;
params.direct = direct;
  
-	r = vm->update_funcs->prepare(, AMDGPU_FENCE_OWNER_KFD, NULL);

+   r = vm->update_funcs->prepare(, NULL, AMDGPU_SYNC_EXPLICIT);
if (r)
return r;
  
@@ -1273,7 +1273,7 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,

params.vm = vm;
params.direct = direct;
  
-	r = vm->update_funcs->prepare(, AMDGPU_FENCE_OWNER_VM, NULL);

+   r = vm->update_funcs->prepare(, NULL, AMDGPU_SYNC_EXPLICIT);
if (r)
return r;
  
@@ -1524,7 +1524,7 @@ static int amdgpu_vm_update_ptes(struct amdgpu_vm_update_params *params,

   * @adev: amdgpu_device pointer
   * @vm: requested vm
   * @direct: direct submission in a page fault
- * @exclusive: fence we need to sync to
+ * @resv: fences we need to sync to
   * @start: start of mapped range
   * @last: last mapped entry
   * @flags: flags for the entries
@@ -1539,14 +1539,14 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_vm_update_params *params,
   */
  static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
   struct amdgpu_vm *vm, bool direct,
-  struct dma_fence *exclusive,
+  struct dma_resv *resv,
   uint64_t start, uint64_t last,
   uint64_t flags, uint64_t addr,
   dma_addr_t *pages_addr,
   struct dma_fence **fence)
  {
struct amdgpu_vm_update_params params;
-   void *owner = AMDGPU_FENCE_OWNER_VM;
+   enum amdgpu_sync_mode sync_mode;
int r;
  
  	memset(, 0, sizeof(params));

@@ -1557,7 +1557,9 @@ static int amdgpu_vm_bo_update_mapping(struct 
amdgpu_device *adev,
  
  	/* sync to everything except eviction fences on unmapping */


I think this comment needs to be updated. Something like "Implicitly 
sync to command submissions in the same VM before unmapping. Sync to 
moving fences before mapping."




if (!(flags & AMDGPU_PTE_VALID))
-   owner = AMDGPU_FENCE_OWNER_KFD;
+   sync_mode = AMDGPU_SYNC_EQ_OWNER;
+   else
+   sync_mode = AMDGPU_SYNC_EXPLICIT;
  
  	mutex_lock(>eviction_lock);

if (vm->evicting) {
@@ -1565,7 +1567,7 @@ static int amdgpu_vm_bo_update_mapping(struct 
amdgpu_device *adev,
goto error_unlock;
}
  
-	r = vm->update_funcs->prepare(, owner, exclusive);

+   r = vm->update_funcs->prepare(, resv, sync_mode);
if (r)
goto error_unlock;
  
@@ -1584,7 +1586,7 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,

   * amdgpu_vm_bo_split_mapping - split a mapping into smaller chunks
   *
   * @adev: amdgpu_device pointer
- * @exclusive: fence we need to sync to
+ * @resv: fences we need to sync to
   * @pages_addr: DMA addresses to use for mapping
   * @vm: requested vm
   * @mapping: mapped range and flags to use for the update
@@ -1600,7 +1602,7 @@ static int amdgpu_vm_bo_update_mapping(struct 
amdgpu_device *adev,
   * 0 for success, -EINVAL for failure.
   */
  static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
-  

Re: [PATCH 10/12] drm/amdgpu: immedially invalidate PTEs

[Felix Kuehling]

On 2020-01-06 10:03 a.m., Christian König wrote:

When a BO is evicted immedially invalidate the mapped PTEs.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 -
  1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index a03cfbe670c4..6844ba7467a6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2569,6 +2569,7 @@ void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
 struct amdgpu_bo *bo, bool evicted)
  {
struct amdgpu_vm_bo_base *bo_base;
+   int r;
  
  	/* shadow bo doesn't have bo base, its validation needs its parent */

if (bo->parent && bo->parent->shadow == bo)
@@ -2576,8 +2577,22 @@ void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
  
  	for (bo_base = bo->vm_bo; bo_base; bo_base = bo_base->next) {

struct amdgpu_vm *vm = bo_base->vm;
+   struct dma_resv *resv = vm->root.base.bo->tbo.base.resv;
+
+   if (bo->tbo.type != ttm_bo_type_kernel) {
+   struct amdgpu_bo_va *bo_va;
+
+   bo_va = container_of(bo_base, struct amdgpu_bo_va,
+base);
+   r = amdgpu_vm_bo_update(adev, bo_va,
+   bo->tbo.base.resv != resv);


Will this update PTEs for per-VM BOs without validating them first?



+   if (!r) {
+   amdgpu_vm_bo_idle(bo_base);


Is this the right state? The description of amdgpu_vm_bo_idle says that 
this is for "PDs/PTs and per VM BOs". For regular BOs, I think this 
should call amdgpu_vm_bo_done.




+   continue;


This skips a bunch of state machine logic below. Maybe some of that 
could be cleaned up.




+   }
+   }
  
-		if (evicted && bo->tbo.base.resv == vm->root.base.bo->tbo.base.resv) {

+   if (evicted && bo->tbo.base.resv == resv) {
amdgpu_vm_bo_evicted(bo_base);


It will never get here for per-VM BOs now (except if bo_update failed). 
Not sure if that's a problem.




continue;
}

 if (bo_base->moved)
 continue;
 bo_base->moved = true;


Maybe the whole PT invalidation should be after this to avoid multiple 
invalidations off the same BO?





 if (bo->tbo.type == ttm_bo_type_kernel)
 amdgpu_vm_bo_relocated(bo_base);
 else if (bo->tbo.resv == vm->root.base.bo->tbo.resv)
 amdgpu_vm_bo_moved(bo_base);
 else
 amdgpu_vm_bo_invalidated(bo_base);


I believe the last two cases are unreachable now (except if bo_update 
failed).


Regards,
  Felix


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

[PATCH] Revert "drm/amdgpu: Set no-retry as default."

[Alex Deucher]

This reverts commit 51bfac71cade386966791a8db87a5912781d249f.

This causes stability issues on some raven boards.  Revert
for now until a proper fix is completed.

Bug: https://gitlab.freedesktop.org/drm/amd/issues/934
Bug: https://bugzilla.kernel.org/show_bug.cgi?id=206017
Signed-off-by: Alex Deucher 
---
 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 0ffc9447b573..3036ec883fb4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -142,7 +142,7 @@ int amdgpu_async_gfx_ring = 1;
 int amdgpu_mcbp = 0;
 int amdgpu_discovery = -1;
 int amdgpu_mes = 0;
-int amdgpu_noretry = 1;
+int amdgpu_noretry;
 int amdgpu_force_asic_type = -1;
 
 struct amdgpu_mgpu_info mgpu_info = {
@@ -588,7 +588,7 @@ MODULE_PARM_DESC(mes,
 module_param_named(mes, amdgpu_mes, int, 0444);
 
 MODULE_PARM_DESC(noretry,
-   "Disable retry faults (0 = retry enabled, 1 = retry disabled 
(default))");
+   "Disable retry faults (0 = retry enabled (default), 1 = retry 
disabled)");
 module_param_named(noretry, amdgpu_noretry, int, 0644);
 
 /**
-- 
2.24.1

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

Re: [PATCH] drm/amdgpu/gfx: simplify old firmware warning

[Christian König]

Am 06.01.20 um 20:58 schrieb Alex Deucher:

Put it on one line to avoid whitespace issues when
printing in the log.

Signed-off-by: Alex Deucher 


Reviewed-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 3 +--
  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  | 3 +--
  2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 5b05334a2548..379e46c1b7f6 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -577,8 +577,7 @@ static void gfx_v10_0_check_fw_write_wait(struct 
amdgpu_device *adev)
}
  
  	if (adev->gfx.cp_fw_write_wait == false)

-   DRM_WARN_ONCE("Warning: check cp_fw_version and update it to 
realize \
- GRBM requires 1-cycle delay in cp firmware\n");
+   DRM_WARN_ONCE("CP firmware version too old, please update!");
  }
  
  
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c

index 35384f543664..1cae356b1ae0 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -988,8 +988,7 @@ static void gfx_v9_0_check_fw_write_wait(struct 
amdgpu_device *adev)
(adev->gfx.mec_feature_version < 46) ||
(adev->gfx.pfp_fw_version < 0x00b7) ||
(adev->gfx.pfp_feature_version < 46))
-   DRM_WARN_ONCE("Warning: check cp_fw_version and update it to 
realize \
- GRBM requires 1-cycle delay in cp firmware\n");
+   DRM_WARN_ONCE("CP firmware version too old, please update!");
  
  	switch (adev->asic_type) {

case CHIP_VEGA10:


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

Re: [PATCH 1/2] drm/amdgpu/gmc: move invaliation bitmap setup to common code

[Christian König]

Am 06.01.20 um 20:16 schrieb Felix Kuehling:

On 2020-01-06 1:35 p.m., Alex Deucher wrote:

So it can be shared with newer GMC versions.

Signed-off-by: Alex Deucher 


Reviewed-by: Felix Kuehling 


Reviewed-by: Christian König 





---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 40 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  1 +
  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 32 +---
  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h   | 10 ---
  4 files changed, 42 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c

index bbcd11ac5bbb..d6901b274790 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -333,3 +333,43 @@ void amdgpu_gmc_ras_fini(struct amdgpu_device 
*adev)

  amdgpu_mmhub_ras_fini(adev);
  amdgpu_xgmi_ras_fini(adev);
  }
+
+    /*
+ * The latest engine allocation on gfx9 is:
+ * Engine 2, 3: firmware
+ * Engine 0, 1, 4~16: amdgpu ring,
+ *    subject to change when ring number changes
+ * Engine 17: Gart flushes
+ */
+#define GFXHUB_FREE_VM_INV_ENGS_BITMAP    0x1FFF3
+#define MMHUB_FREE_VM_INV_ENGS_BITMAP    0x1FFF3
+
+int amdgpu_gmc_allocate_vm_inv_eng(struct amdgpu_device *adev)
+{
+    struct amdgpu_ring *ring;
+    unsigned vm_inv_engs[AMDGPU_MAX_VMHUBS] =
+    {GFXHUB_FREE_VM_INV_ENGS_BITMAP, MMHUB_FREE_VM_INV_ENGS_BITMAP,
+    GFXHUB_FREE_VM_INV_ENGS_BITMAP};
+    unsigned i;
+    unsigned vmhub, inv_eng;
+
+    for (i = 0; i < adev->num_rings; ++i) {
+    ring = adev->rings[i];
+    vmhub = ring->funcs->vmhub;
+
+    inv_eng = ffs(vm_inv_engs[vmhub]);
+    if (!inv_eng) {
+    dev_err(adev->dev, "no VM inv eng for ring %s\n",
+    ring->name);
+    return -EINVAL;
+    }
+
+    ring->vm_inv_eng = inv_eng - 1;
+    vm_inv_engs[vmhub] &= ~(1 << ring->vm_inv_eng);
+
+    dev_info(adev->dev, "ring %s uses VM inv eng %u on hub %u\n",
+ ring->name, ring->vm_inv_eng, ring->funcs->vmhub);
+    }
+
+    return 0;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h

index b499a3de8bb6..c91dd602d5f1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -267,5 +267,6 @@ bool amdgpu_gmc_filter_faults(struct 
amdgpu_device *adev, uint64_t addr,

    uint16_t pasid, uint64_t timestamp);
  int amdgpu_gmc_ras_late_init(struct amdgpu_device *adev);
  void amdgpu_gmc_ras_fini(struct amdgpu_device *adev);
+int amdgpu_gmc_allocate_vm_inv_eng(struct amdgpu_device *adev);
    #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c

index 68f9a1fa6dc1..e3bbeab28152 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -795,36 +795,6 @@ static bool gmc_v9_0_keep_stolen_memory(struct 
amdgpu_device *adev)

  }
  }
  -static int gmc_v9_0_allocate_vm_inv_eng(struct amdgpu_device *adev)
-{
-    struct amdgpu_ring *ring;
-    unsigned vm_inv_engs[AMDGPU_MAX_VMHUBS] =
-    {GFXHUB_FREE_VM_INV_ENGS_BITMAP, MMHUB_FREE_VM_INV_ENGS_BITMAP,
-    GFXHUB_FREE_VM_INV_ENGS_BITMAP};
-    unsigned i;
-    unsigned vmhub, inv_eng;
-
-    for (i = 0; i < adev->num_rings; ++i) {
-    ring = adev->rings[i];
-    vmhub = ring->funcs->vmhub;
-
-    inv_eng = ffs(vm_inv_engs[vmhub]);
-    if (!inv_eng) {
-    dev_err(adev->dev, "no VM inv eng for ring %s\n",
-    ring->name);
-    return -EINVAL;
-    }
-
-    ring->vm_inv_eng = inv_eng - 1;
-    vm_inv_engs[vmhub] &= ~(1 << ring->vm_inv_eng);
-
-    dev_info(adev->dev, "ring %s uses VM inv eng %u on hub %u\n",
- ring->name, ring->vm_inv_eng, ring->funcs->vmhub);
-    }
-
-    return 0;
-}
-
  static int gmc_v9_0_late_init(void *handle)
  {
  struct amdgpu_device *adev = (struct amdgpu_device *)handle;
@@ -833,7 +803,7 @@ static int gmc_v9_0_late_init(void *handle)
  if (!gmc_v9_0_keep_stolen_memory(adev))
  amdgpu_bo_late_init(adev);
  -    r = gmc_v9_0_allocate_vm_inv_eng(adev);
+    r = amdgpu_gmc_allocate_vm_inv_eng(adev);
  if (r)
  return r;
  /* Check if ecc is available */
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h

index 49e8be761214..e0585e8c6c1b 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h
@@ -24,16 +24,6 @@
  #ifndef __GMC_V9_0_H__
  #define __GMC_V9_0_H__
  -    /*
- * The latest engine allocation on gfx9 is:
- * Engine 2, 3: firmware
- * Engine 0, 1, 4~16: amdgpu ring,
- *    subject to change when ring number changes
- * Engine 17: Gart flushes
- */
-#define GFXHUB_FREE_VM_INV_ENGS_BITMAP    0x1FFF3
-#define 

Re: [PATCH 2/2] drm/amdgpu/gmc10: use common invalidation engine helper

[Christian König]

No idea either why that was done.

Anyway patch is Acked-by: Christian König .

Regards,
Christian.

Am 06.01.20 um 20:16 schrieb Felix Kuehling:

This patch is

Acked-by: Felix Kuehling 

I don't really know why the mask was different on GFXv10. I suspect it 
was laziness/simplicity of not having the gap at engines 2,3. If we 
don't have as many rings on GFXv10 ASICs we probably don't need 
engines 0,1.


Regards,
  Felix

On 2020-01-06 1:35 p.m., Alex Deucher wrote:

Rather than open coding it.  This also changes the free masks
to better reflect the usage by other components.

Signed-off-by: Alex Deucher 
---

We always started at invalidation engine 4 on gmc10.  Was there a
reason for that?  I would have figured it would follow the same
model as older asics.  I can make this more flexiable if so.


  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c |  2 +-
  drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 19 ---
  2 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c

index d6901b274790..5884ab590486 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -335,7 +335,7 @@ void amdgpu_gmc_ras_fini(struct amdgpu_device *adev)
  }
    /*
- * The latest engine allocation on gfx9 is:
+ * The latest engine allocation on gfx9/10 is:
   * Engine 2, 3: firmware
   * Engine 0, 1, 4~16: amdgpu ring,
   *    subject to change when ring number changes
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c

index f5725336a5f2..da9765ff45d6 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -564,22 +564,11 @@ static int gmc_v10_0_early_init(void *handle)
  static int gmc_v10_0_late_init(void *handle)
  {
  struct amdgpu_device *adev = (struct amdgpu_device *)handle;
-    unsigned vm_inv_eng[AMDGPU_MAX_VMHUBS] = { 4, 4 };
-    unsigned i;
-
-    for(i = 0; i < adev->num_rings; ++i) {
-    struct amdgpu_ring *ring = adev->rings[i];
-    unsigned vmhub = ring->funcs->vmhub;
-
-    ring->vm_inv_eng = vm_inv_eng[vmhub]++;
-    dev_info(adev->dev, "ring %u(%s) uses VM inv eng %u on hub 
%u\n",

- ring->idx, ring->name, ring->vm_inv_eng,
- ring->funcs->vmhub);
-    }
+    int r;
  -    /* Engine 17 is used for GART flushes */
-    for(i = 0; i < AMDGPU_MAX_VMHUBS; ++i)
-    BUG_ON(vm_inv_eng[i] > 17);
+    r = amdgpu_gmc_allocate_vm_inv_eng(adev);
+    if (r)
+    return r;
    return amdgpu_irq_get(adev, >gmc.vm_fault, 0);
  }

___
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

Problem whit Radeon rx 590 makes games crash on Linux

[Martin Bångens]

Hi AMD graphic driver developer for Linux!

I have problems with playing games using Linux opensource driver

I have xfx radeon rx 590 fatboy and tested with proprietary driver games 
run fine but too slow for playing


here is bit more info about my experience

ask me anything


https://community.amd.com/thread/246894

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

[PATCH] drm/amdgpu/gfx: simplify old firmware warning

[Alex Deucher]

Put it on one line to avoid whitespace issues when
printing in the log.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 3 +--
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 5b05334a2548..379e46c1b7f6 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -577,8 +577,7 @@ static void gfx_v10_0_check_fw_write_wait(struct 
amdgpu_device *adev)
}
 
if (adev->gfx.cp_fw_write_wait == false)
-   DRM_WARN_ONCE("Warning: check cp_fw_version and update it to 
realize \
- GRBM requires 1-cycle delay in cp firmware\n");
+   DRM_WARN_ONCE("CP firmware version too old, please update!");
 }
 
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 35384f543664..1cae356b1ae0 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -988,8 +988,7 @@ static void gfx_v9_0_check_fw_write_wait(struct 
amdgpu_device *adev)
(adev->gfx.mec_feature_version < 46) ||
(adev->gfx.pfp_fw_version < 0x00b7) ||
(adev->gfx.pfp_feature_version < 46))
-   DRM_WARN_ONCE("Warning: check cp_fw_version and update it to 
realize \
- GRBM requires 1-cycle delay in cp firmware\n");
+   DRM_WARN_ONCE("CP firmware version too old, please update!");
 
switch (adev->asic_type) {
case CHIP_VEGA10:
-- 
2.24.1

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

Re: [PATCH 1/2] drm/amdgpu/gmc: move invaliation bitmap setup to common code

[Felix Kuehling]

On 2020-01-06 1:35 p.m., Alex Deucher wrote:

So it can be shared with newer GMC versions.

Signed-off-by: Alex Deucher 


Reviewed-by: Felix Kuehling 



---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 40 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  1 +
  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 32 +---
  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h   | 10 ---
  4 files changed, 42 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index bbcd11ac5bbb..d6901b274790 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -333,3 +333,43 @@ void amdgpu_gmc_ras_fini(struct amdgpu_device *adev)
amdgpu_mmhub_ras_fini(adev);
amdgpu_xgmi_ras_fini(adev);
  }
+
+   /*
+* The latest engine allocation on gfx9 is:
+* Engine 2, 3: firmware
+* Engine 0, 1, 4~16: amdgpu ring,
+*subject to change when ring number changes
+* Engine 17: Gart flushes
+*/
+#define GFXHUB_FREE_VM_INV_ENGS_BITMAP 0x1FFF3
+#define MMHUB_FREE_VM_INV_ENGS_BITMAP  0x1FFF3
+
+int amdgpu_gmc_allocate_vm_inv_eng(struct amdgpu_device *adev)
+{
+   struct amdgpu_ring *ring;
+   unsigned vm_inv_engs[AMDGPU_MAX_VMHUBS] =
+   {GFXHUB_FREE_VM_INV_ENGS_BITMAP, MMHUB_FREE_VM_INV_ENGS_BITMAP,
+   GFXHUB_FREE_VM_INV_ENGS_BITMAP};
+   unsigned i;
+   unsigned vmhub, inv_eng;
+
+   for (i = 0; i < adev->num_rings; ++i) {
+   ring = adev->rings[i];
+   vmhub = ring->funcs->vmhub;
+
+   inv_eng = ffs(vm_inv_engs[vmhub]);
+   if (!inv_eng) {
+   dev_err(adev->dev, "no VM inv eng for ring %s\n",
+   ring->name);
+   return -EINVAL;
+   }
+
+   ring->vm_inv_eng = inv_eng - 1;
+   vm_inv_engs[vmhub] &= ~(1 << ring->vm_inv_eng);
+
+   dev_info(adev->dev, "ring %s uses VM inv eng %u on hub %u\n",
+ring->name, ring->vm_inv_eng, ring->funcs->vmhub);
+   }
+
+   return 0;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index b499a3de8bb6..c91dd602d5f1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -267,5 +267,6 @@ bool amdgpu_gmc_filter_faults(struct amdgpu_device *adev, 
uint64_t addr,
  uint16_t pasid, uint64_t timestamp);
  int amdgpu_gmc_ras_late_init(struct amdgpu_device *adev);
  void amdgpu_gmc_ras_fini(struct amdgpu_device *adev);
+int amdgpu_gmc_allocate_vm_inv_eng(struct amdgpu_device *adev);
  
  #endif

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 68f9a1fa6dc1..e3bbeab28152 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -795,36 +795,6 @@ static bool gmc_v9_0_keep_stolen_memory(struct 
amdgpu_device *adev)
}
  }
  
-static int gmc_v9_0_allocate_vm_inv_eng(struct amdgpu_device *adev)

-{
-   struct amdgpu_ring *ring;
-   unsigned vm_inv_engs[AMDGPU_MAX_VMHUBS] =
-   {GFXHUB_FREE_VM_INV_ENGS_BITMAP, MMHUB_FREE_VM_INV_ENGS_BITMAP,
-   GFXHUB_FREE_VM_INV_ENGS_BITMAP};
-   unsigned i;
-   unsigned vmhub, inv_eng;
-
-   for (i = 0; i < adev->num_rings; ++i) {
-   ring = adev->rings[i];
-   vmhub = ring->funcs->vmhub;
-
-   inv_eng = ffs(vm_inv_engs[vmhub]);
-   if (!inv_eng) {
-   dev_err(adev->dev, "no VM inv eng for ring %s\n",
-   ring->name);
-   return -EINVAL;
-   }
-
-   ring->vm_inv_eng = inv_eng - 1;
-   vm_inv_engs[vmhub] &= ~(1 << ring->vm_inv_eng);
-
-   dev_info(adev->dev, "ring %s uses VM inv eng %u on hub %u\n",
-ring->name, ring->vm_inv_eng, ring->funcs->vmhub);
-   }
-
-   return 0;
-}
-
  static int gmc_v9_0_late_init(void *handle)
  {
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
@@ -833,7 +803,7 @@ static int gmc_v9_0_late_init(void *handle)
if (!gmc_v9_0_keep_stolen_memory(adev))
amdgpu_bo_late_init(adev);
  
-	r = gmc_v9_0_allocate_vm_inv_eng(adev);

+   r = amdgpu_gmc_allocate_vm_inv_eng(adev);
if (r)
return r;
/* Check if ecc is available */
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h
index 49e8be761214..e0585e8c6c1b 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h
@@ -24,16 +24,6 @@
  #ifndef __GMC_V9_0_H__
  #define __GMC_V9_0_H__
  
-	/*

-* The latest engine allocation on gfx9 is:
-

Re: [PATCH 2/2] drm/amdgpu/gmc10: use common invalidation engine helper

[Felix Kuehling]

This patch is

Acked-by: Felix Kuehling 

I don't really know why the mask was different on GFXv10. I suspect it 
was laziness/simplicity of not having the gap at engines 2,3. If we 
don't have as many rings on GFXv10 ASICs we probably don't need engines 0,1.


Regards,
  Felix

On 2020-01-06 1:35 p.m., Alex Deucher wrote:

Rather than open coding it.  This also changes the free masks
to better reflect the usage by other components.

Signed-off-by: Alex Deucher 
---

We always started at invalidation engine 4 on gmc10.  Was there a
reason for that?  I would have figured it would follow the same
model as older asics.  I can make this more flexiable if so.


  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c |  2 +-
  drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 19 ---
  2 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index d6901b274790..5884ab590486 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -335,7 +335,7 @@ void amdgpu_gmc_ras_fini(struct amdgpu_device *adev)
  }
  
  	/*

-* The latest engine allocation on gfx9 is:
+* The latest engine allocation on gfx9/10 is:
 * Engine 2, 3: firmware
 * Engine 0, 1, 4~16: amdgpu ring,
 *subject to change when ring number changes
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index f5725336a5f2..da9765ff45d6 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -564,22 +564,11 @@ static int gmc_v10_0_early_init(void *handle)
  static int gmc_v10_0_late_init(void *handle)
  {
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
-   unsigned vm_inv_eng[AMDGPU_MAX_VMHUBS] = { 4, 4 };
-   unsigned i;
-
-   for(i = 0; i < adev->num_rings; ++i) {
-   struct amdgpu_ring *ring = adev->rings[i];
-   unsigned vmhub = ring->funcs->vmhub;
-
-   ring->vm_inv_eng = vm_inv_eng[vmhub]++;
-   dev_info(adev->dev, "ring %u(%s) uses VM inv eng %u on hub 
%u\n",
-ring->idx, ring->name, ring->vm_inv_eng,
-ring->funcs->vmhub);
-   }
+   int r;
  
-	/* Engine 17 is used for GART flushes */

-   for(i = 0; i < AMDGPU_MAX_VMHUBS; ++i)
-   BUG_ON(vm_inv_eng[i] > 17);
+   r = amdgpu_gmc_allocate_vm_inv_eng(adev);
+   if (r)
+   return r;
  
  	return amdgpu_irq_get(adev, >gmc.vm_fault, 0);

  }

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

[PATCH 2/2] drm/amdgpu/gmc10: use common invalidation engine helper

[Alex Deucher]

Rather than open coding it.  This also changes the free masks
to better reflect the usage by other components.

Signed-off-by: Alex Deucher 
---

We always started at invalidation engine 4 on gmc10.  Was there a
reason for that?  I would have figured it would follow the same
model as older asics.  I can make this more flexiable if so.


 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 19 ---
 2 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index d6901b274790..5884ab590486 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -335,7 +335,7 @@ void amdgpu_gmc_ras_fini(struct amdgpu_device *adev)
 }
 
/*
-* The latest engine allocation on gfx9 is:
+* The latest engine allocation on gfx9/10 is:
 * Engine 2, 3: firmware
 * Engine 0, 1, 4~16: amdgpu ring,
 *subject to change when ring number changes
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index f5725336a5f2..da9765ff45d6 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -564,22 +564,11 @@ static int gmc_v10_0_early_init(void *handle)
 static int gmc_v10_0_late_init(void *handle)
 {
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
-   unsigned vm_inv_eng[AMDGPU_MAX_VMHUBS] = { 4, 4 };
-   unsigned i;
-
-   for(i = 0; i < adev->num_rings; ++i) {
-   struct amdgpu_ring *ring = adev->rings[i];
-   unsigned vmhub = ring->funcs->vmhub;
-
-   ring->vm_inv_eng = vm_inv_eng[vmhub]++;
-   dev_info(adev->dev, "ring %u(%s) uses VM inv eng %u on hub 
%u\n",
-ring->idx, ring->name, ring->vm_inv_eng,
-ring->funcs->vmhub);
-   }
+   int r;
 
-   /* Engine 17 is used for GART flushes */
-   for(i = 0; i < AMDGPU_MAX_VMHUBS; ++i)
-   BUG_ON(vm_inv_eng[i] > 17);
+   r = amdgpu_gmc_allocate_vm_inv_eng(adev);
+   if (r)
+   return r;
 
return amdgpu_irq_get(adev, >gmc.vm_fault, 0);
 }
-- 
2.24.1

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

[PATCH 1/2] drm/amdgpu/gmc: move invaliation bitmap setup to common code

[Alex Deucher]

So it can be shared with newer GMC versions.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 40 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  1 +
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 32 +---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h   | 10 ---
 4 files changed, 42 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index bbcd11ac5bbb..d6901b274790 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -333,3 +333,43 @@ void amdgpu_gmc_ras_fini(struct amdgpu_device *adev)
amdgpu_mmhub_ras_fini(adev);
amdgpu_xgmi_ras_fini(adev);
 }
+
+   /*
+* The latest engine allocation on gfx9 is:
+* Engine 2, 3: firmware
+* Engine 0, 1, 4~16: amdgpu ring,
+*subject to change when ring number changes
+* Engine 17: Gart flushes
+*/
+#define GFXHUB_FREE_VM_INV_ENGS_BITMAP 0x1FFF3
+#define MMHUB_FREE_VM_INV_ENGS_BITMAP  0x1FFF3
+
+int amdgpu_gmc_allocate_vm_inv_eng(struct amdgpu_device *adev)
+{
+   struct amdgpu_ring *ring;
+   unsigned vm_inv_engs[AMDGPU_MAX_VMHUBS] =
+   {GFXHUB_FREE_VM_INV_ENGS_BITMAP, MMHUB_FREE_VM_INV_ENGS_BITMAP,
+   GFXHUB_FREE_VM_INV_ENGS_BITMAP};
+   unsigned i;
+   unsigned vmhub, inv_eng;
+
+   for (i = 0; i < adev->num_rings; ++i) {
+   ring = adev->rings[i];
+   vmhub = ring->funcs->vmhub;
+
+   inv_eng = ffs(vm_inv_engs[vmhub]);
+   if (!inv_eng) {
+   dev_err(adev->dev, "no VM inv eng for ring %s\n",
+   ring->name);
+   return -EINVAL;
+   }
+
+   ring->vm_inv_eng = inv_eng - 1;
+   vm_inv_engs[vmhub] &= ~(1 << ring->vm_inv_eng);
+
+   dev_info(adev->dev, "ring %s uses VM inv eng %u on hub %u\n",
+ring->name, ring->vm_inv_eng, ring->funcs->vmhub);
+   }
+
+   return 0;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index b499a3de8bb6..c91dd602d5f1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -267,5 +267,6 @@ bool amdgpu_gmc_filter_faults(struct amdgpu_device *adev, 
uint64_t addr,
  uint16_t pasid, uint64_t timestamp);
 int amdgpu_gmc_ras_late_init(struct amdgpu_device *adev);
 void amdgpu_gmc_ras_fini(struct amdgpu_device *adev);
+int amdgpu_gmc_allocate_vm_inv_eng(struct amdgpu_device *adev);
 
 #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 68f9a1fa6dc1..e3bbeab28152 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -795,36 +795,6 @@ static bool gmc_v9_0_keep_stolen_memory(struct 
amdgpu_device *adev)
}
 }
 
-static int gmc_v9_0_allocate_vm_inv_eng(struct amdgpu_device *adev)
-{
-   struct amdgpu_ring *ring;
-   unsigned vm_inv_engs[AMDGPU_MAX_VMHUBS] =
-   {GFXHUB_FREE_VM_INV_ENGS_BITMAP, MMHUB_FREE_VM_INV_ENGS_BITMAP,
-   GFXHUB_FREE_VM_INV_ENGS_BITMAP};
-   unsigned i;
-   unsigned vmhub, inv_eng;
-
-   for (i = 0; i < adev->num_rings; ++i) {
-   ring = adev->rings[i];
-   vmhub = ring->funcs->vmhub;
-
-   inv_eng = ffs(vm_inv_engs[vmhub]);
-   if (!inv_eng) {
-   dev_err(adev->dev, "no VM inv eng for ring %s\n",
-   ring->name);
-   return -EINVAL;
-   }
-
-   ring->vm_inv_eng = inv_eng - 1;
-   vm_inv_engs[vmhub] &= ~(1 << ring->vm_inv_eng);
-
-   dev_info(adev->dev, "ring %s uses VM inv eng %u on hub %u\n",
-ring->name, ring->vm_inv_eng, ring->funcs->vmhub);
-   }
-
-   return 0;
-}
-
 static int gmc_v9_0_late_init(void *handle)
 {
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
@@ -833,7 +803,7 @@ static int gmc_v9_0_late_init(void *handle)
if (!gmc_v9_0_keep_stolen_memory(adev))
amdgpu_bo_late_init(adev);
 
-   r = gmc_v9_0_allocate_vm_inv_eng(adev);
+   r = amdgpu_gmc_allocate_vm_inv_eng(adev);
if (r)
return r;
/* Check if ecc is available */
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h
index 49e8be761214..e0585e8c6c1b 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h
@@ -24,16 +24,6 @@
 #ifndef __GMC_V9_0_H__
 #define __GMC_V9_0_H__
 
-   /*
-* The latest engine allocation on gfx9 is:
-* Engine 2, 3: firmware
-* Engine 0, 1, 4~16: amdgpu ring,
-*  

Re: [PATCH 2/5] drm/amdgpu: export function to flush TLB via pasid

[Christian König]

Am 06.01.20 um 17:04 schrieb Felix Kuehling:

On 2020-01-05 10:41 a.m., Christian König wrote:

Am 20.12.19 um 07:24 schrieb Alex Sierra:

This can be used directly from amdgpu and amdkfd to invalidate
TLB through pasid.
It supports gmc v7, v8, v9 and v10.

Change-Id: I6563a8eba2e42d1a67fa2547156c20da41d1e490
Signed-off-by: Alex Sierra 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  6 ++
  drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 81 
  drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c   | 33 ++
  drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   | 34 ++
  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 84 
+

  5 files changed, 238 insertions(+)

[snip]
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c

index fa025ceeea0f..eb1e64bd56ed 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -38,10 +38,12 @@
  #include "dce/dce_12_0_sh_mask.h"
  #include "vega10_enum.h"
  #include "mmhub/mmhub_1_0_offset.h"
+#include "athub/athub_1_0_sh_mask.h"
  #include "athub/athub_1_0_offset.h"
  #include "oss/osssys_4_0_offset.h"
    #include "soc15.h"
+#include "soc15d.h"
  #include "soc15_common.h"
  #include "umc/umc_6_0_sh_mask.h"
  @@ -434,6 +436,47 @@ static bool 
gmc_v9_0_use_invalidate_semaphore(struct amdgpu_device *adev,

 adev->pdev->device == 0x15d8)));
  }
  +static bool gmc_v9_0_get_atc_vmid_pasid_mapping_info(struct 
amdgpu_device *adev,

+    uint8_t vmid, uint16_t *p_pasid)
+{
+    uint32_t value;
+
+    value = RREG32(SOC15_REG_OFFSET(ATHUB, 0, 
mmATC_VMID0_PASID_MAPPING)

+ + vmid);
+    *p_pasid = value & ATC_VMID0_PASID_MAPPING__PASID_MASK;
+
+    return !!(value & ATC_VMID0_PASID_MAPPING__VALID_MASK);
+}


Probably better to expose just this function in the GMC interface.


This is called below in gmc_v9_0_flush_gpu_tlb_pasid in the case that 
the KIQ is not ready. It is not needed outside this file, so no need 
to expose it in the GMC interface.






+
+static int gmc_v9_0_invalidate_tlbs_with_kiq(struct amdgpu_device 
*adev,

+    uint16_t pasid, uint32_t flush_type,
+    bool all_hub)
+{
+    signed long r;
+    uint32_t seq;
+    struct amdgpu_ring *ring = >gfx.kiq.ring;
+
+    spin_lock(>gfx.kiq.ring_lock);
+    amdgpu_ring_alloc(ring, 12); /* fence + invalidate_tlbs package*/
+    amdgpu_ring_write(ring, PACKET3(PACKET3_INVALIDATE_TLBS, 0));
+    amdgpu_ring_write(ring,
+    PACKET3_INVALIDATE_TLBS_DST_SEL(1) |
+    PACKET3_INVALIDATE_TLBS_ALL_HUB(all_hub) |
+    PACKET3_INVALIDATE_TLBS_PASID(pasid) |
+    PACKET3_INVALIDATE_TLBS_FLUSH_TYPE(flush_type));


That stuff is completely unrelated to the GMC and shouldn't be added 
here.


Where else should it go? To me TLB flushing is very much something 
that belongs in this file.


Maybe you'd rather add "flush_tlbs_with_kiq" to amdgpu_ring_funcs and 
implement it in the gfx_v*.c GFX-IP code? I'm not sure that makes much 
sense either because it's only available on the KIQ ring.


Yes, something like this. We should probably add a amdgpu_kiq_funcs and 
expose the functions needed by the GMC code there.


See the amdgpu_virt_kiq_reg_write_reg_wait() case for reference, it is 
very similar and should probably be added to that function table as well.


Christian.






Christian.


+    amdgpu_fence_emit_polling(ring, );
+    amdgpu_ring_commit(ring);
+    spin_unlock(>gfx.kiq.ring_lock);
+
+    r = amdgpu_fence_wait_polling(ring, seq, adev->usec_timeout);
+    if (r < 1) {
+    DRM_ERROR("wait for kiq fence error: %ld.\n", r);
+    return -ETIME;
+    }
+
+    return 0;
+}
+
  /*
   * GART
   * VMID 0 is the physical GPU addresses as used by the kernel.
@@ -532,6 +575,46 @@ static void gmc_v9_0_flush_gpu_tlb(struct 
amdgpu_device *adev, uint32_t vmid,

  DRM_ERROR("Timeout waiting for VM flush ACK!\n");
  }
  +/**
+ * gmc_v9_0_flush_gpu_tlb_pasid - tlb flush via pasid
+ *
+ * @adev: amdgpu_device pointer
+ * @pasid: pasid to be flush
+ *
+ * Flush the TLB for the requested pasid.
+ */
+static int gmc_v9_0_flush_gpu_tlb_pasid(struct amdgpu_device *adev,
+    uint16_t pasid, uint32_t flush_type,
+    bool all_hub)


Christian, do you agree that this function belongs in the GMC 
interface? If not here, where do you suggest moving it?


Regards,
  Felix



+{
+    int vmid, i;
+    uint16_t queried_pasid;
+    bool ret;
+    struct amdgpu_ring *ring = >gfx.kiq.ring;
+
+    if (adev->in_gpu_reset)
+    return -EIO;
+
+    if (ring->sched.ready)
+    return gmc_v9_0_invalidate_tlbs_with_kiq(adev,
+    pasid, flush_type, all_hub);
+
+    for (vmid = 1; vmid < 16; vmid++) {
+
+    ret = gmc_v9_0_get_atc_vmid_pasid_mapping_info(adev, vmid,
+    _pasid);
+    if (ret && queried_pasid == pasid) {
+    for (i = 0; i < adev->num_vmhubs; i++)
+  

Re: [PATCH 2/5] drm/amdgpu: export function to flush TLB via pasid

[Felix Kuehling]

On 2020-01-05 10:41 a.m., Christian König wrote:

Am 20.12.19 um 07:24 schrieb Alex Sierra:

This can be used directly from amdgpu and amdkfd to invalidate
TLB through pasid.
It supports gmc v7, v8, v9 and v10.

Change-Id: I6563a8eba2e42d1a67fa2547156c20da41d1e490
Signed-off-by: Alex Sierra 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  6 ++
  drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 81 
  drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c   | 33 ++
  drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   | 34 ++
  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 84 +
  5 files changed, 238 insertions(+)

[snip]
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c

index fa025ceeea0f..eb1e64bd56ed 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -38,10 +38,12 @@
  #include "dce/dce_12_0_sh_mask.h"
  #include "vega10_enum.h"
  #include "mmhub/mmhub_1_0_offset.h"
+#include "athub/athub_1_0_sh_mask.h"
  #include "athub/athub_1_0_offset.h"
  #include "oss/osssys_4_0_offset.h"
    #include "soc15.h"
+#include "soc15d.h"
  #include "soc15_common.h"
  #include "umc/umc_6_0_sh_mask.h"
  @@ -434,6 +436,47 @@ static bool 
gmc_v9_0_use_invalidate_semaphore(struct amdgpu_device *adev,

 adev->pdev->device == 0x15d8)));
  }
  +static bool gmc_v9_0_get_atc_vmid_pasid_mapping_info(struct 
amdgpu_device *adev,

+    uint8_t vmid, uint16_t *p_pasid)
+{
+    uint32_t value;
+
+    value = RREG32(SOC15_REG_OFFSET(ATHUB, 0, 
mmATC_VMID0_PASID_MAPPING)

+ + vmid);
+    *p_pasid = value & ATC_VMID0_PASID_MAPPING__PASID_MASK;
+
+    return !!(value & ATC_VMID0_PASID_MAPPING__VALID_MASK);
+}


Probably better to expose just this function in the GMC interface.


This is called below in gmc_v9_0_flush_gpu_tlb_pasid in the case that 
the KIQ is not ready. It is not needed outside this file, so no need to 
expose it in the GMC interface.






+
+static int gmc_v9_0_invalidate_tlbs_with_kiq(struct amdgpu_device 
*adev,

+    uint16_t pasid, uint32_t flush_type,
+    bool all_hub)
+{
+    signed long r;
+    uint32_t seq;
+    struct amdgpu_ring *ring = >gfx.kiq.ring;
+
+    spin_lock(>gfx.kiq.ring_lock);
+    amdgpu_ring_alloc(ring, 12); /* fence + invalidate_tlbs package*/
+    amdgpu_ring_write(ring, PACKET3(PACKET3_INVALIDATE_TLBS, 0));
+    amdgpu_ring_write(ring,
+    PACKET3_INVALIDATE_TLBS_DST_SEL(1) |
+    PACKET3_INVALIDATE_TLBS_ALL_HUB(all_hub) |
+    PACKET3_INVALIDATE_TLBS_PASID(pasid) |
+    PACKET3_INVALIDATE_TLBS_FLUSH_TYPE(flush_type));


That stuff is completely unrelated to the GMC and shouldn't be added 
here.


Where else should it go? To me TLB flushing is very much something that 
belongs in this file.


Maybe you'd rather add "flush_tlbs_with_kiq" to amdgpu_ring_funcs and 
implement it in the gfx_v*.c GFX-IP code? I'm not sure that makes much 
sense either because it's only available on the KIQ ring.





Christian.


+    amdgpu_fence_emit_polling(ring, );
+    amdgpu_ring_commit(ring);
+    spin_unlock(>gfx.kiq.ring_lock);
+
+    r = amdgpu_fence_wait_polling(ring, seq, adev->usec_timeout);
+    if (r < 1) {
+    DRM_ERROR("wait for kiq fence error: %ld.\n", r);
+    return -ETIME;
+    }
+
+    return 0;
+}
+
  /*
   * GART
   * VMID 0 is the physical GPU addresses as used by the kernel.
@@ -532,6 +575,46 @@ static void gmc_v9_0_flush_gpu_tlb(struct 
amdgpu_device *adev, uint32_t vmid,

  DRM_ERROR("Timeout waiting for VM flush ACK!\n");
  }
  +/**
+ * gmc_v9_0_flush_gpu_tlb_pasid - tlb flush via pasid
+ *
+ * @adev: amdgpu_device pointer
+ * @pasid: pasid to be flush
+ *
+ * Flush the TLB for the requested pasid.
+ */
+static int gmc_v9_0_flush_gpu_tlb_pasid(struct amdgpu_device *adev,
+    uint16_t pasid, uint32_t flush_type,
+    bool all_hub)


Christian, do you agree that this function belongs in the GMC interface? 
If not here, where do you suggest moving it?


Regards,
  Felix



+{
+    int vmid, i;
+    uint16_t queried_pasid;
+    bool ret;
+    struct amdgpu_ring *ring = >gfx.kiq.ring;
+
+    if (adev->in_gpu_reset)
+    return -EIO;
+
+    if (ring->sched.ready)
+    return gmc_v9_0_invalidate_tlbs_with_kiq(adev,
+    pasid, flush_type, all_hub);
+
+    for (vmid = 1; vmid < 16; vmid++) {
+
+    ret = gmc_v9_0_get_atc_vmid_pasid_mapping_info(adev, vmid,
+    _pasid);
+    if (ret && queried_pasid == pasid) {
+    for (i = 0; i < adev->num_vmhubs; i++)
+    amdgpu_gmc_flush_gpu_tlb(adev, vmid,
+    i, flush_type);
+    break;
+    }
+    }
+
+    return 0;
+
+}
+
  static uint64_t gmc_v9_0_emit_flush_gpu_tlb(struct amdgpu_ring *ring,
  unsigned vmid, uint64_t pd_addr)
  {
@@ -693,6 +776,7 @@ static void 

[PATCH 03/12] drm/amdgpu: add VM eviction lock v3

[Christian König]

This allows to invalidate VM entries without taking the reservation lock.

v3: use -EBUSY

Signed-off-by: Christian König 
Reviewed-by: Felix Kuehling 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 39 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  4 +++
 2 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 0d700e8154c4..54430c9f7a27 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -656,7 +656,7 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,
  void *param)
 {
struct amdgpu_vm_bo_base *bo_base, *tmp;
-   int r = 0;
+   int r;
 
vm->bulk_moveable &= list_empty(>evicted);
 
@@ -665,7 +665,7 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,
 
r = validate(param, bo);
if (r)
-   break;
+   return r;
 
if (bo->tbo.type != ttm_bo_type_kernel) {
amdgpu_vm_bo_moved(bo_base);
@@ -678,7 +678,11 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,
}
}
 
-   return r;
+   mutex_lock(>eviction_lock);
+   vm->evicting = false;
+   mutex_unlock(>eviction_lock);
+
+   return 0;
 }
 
 /**
@@ -1555,15 +1559,25 @@ static int amdgpu_vm_bo_update_mapping(struct 
amdgpu_device *adev,
if (!(flags & AMDGPU_PTE_VALID))
owner = AMDGPU_FENCE_OWNER_KFD;
 
+   mutex_lock(>eviction_lock);
+   if (vm->evicting) {
+   r = -EBUSY;
+   goto error_unlock;
+   }
+
r = vm->update_funcs->prepare(, owner, exclusive);
if (r)
-   return r;
+   goto error_unlock;
 
r = amdgpu_vm_update_ptes(, start, last + 1, addr, flags);
if (r)
-   return r;
+   goto error_unlock;
 
-   return vm->update_funcs->commit(, fence);
+   r = vm->update_funcs->commit(, fence);
+
+error_unlock:
+   mutex_unlock(>eviction_lock);
+   return r;
 }
 
 /**
@@ -2522,11 +2536,19 @@ bool amdgpu_vm_evictable(struct amdgpu_bo *bo)
if (!dma_resv_test_signaled_rcu(bo->tbo.base.resv, true))
return false;
 
+   /* Try to block ongoing updates */
+   if (!mutex_trylock(_base->vm->eviction_lock))
+   return false;
+
/* Don't evict VM page tables while they are updated */
if (!dma_fence_is_signaled(bo_base->vm->last_direct) ||
-   !dma_fence_is_signaled(bo_base->vm->last_delayed))
+   !dma_fence_is_signaled(bo_base->vm->last_delayed)) {
+   mutex_unlock(_base->vm->eviction_lock);
return false;
+   }
 
+   bo_base->vm->evicting = true;
+   mutex_unlock(_base->vm->eviction_lock);
return true;
 }
 
@@ -2773,6 +2795,9 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct 
amdgpu_vm *vm,
vm->last_direct = dma_fence_get_stub();
vm->last_delayed = dma_fence_get_stub();
 
+   mutex_init(>eviction_lock);
+   vm->evicting = false;
+
amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, false, );
if (vm_context == AMDGPU_VM_CONTEXT_COMPUTE)
bp.flags &= ~AMDGPU_GEM_CREATE_SHADOW;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index d93ea9ad879e..d5613d184e99 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -242,6 +242,10 @@ struct amdgpu_vm {
/* tree of virtual addresses mapped */
struct rb_root_cached   va;
 
+   /* Lock to prevent eviction while we are updating page tables */
+   struct mutexeviction_lock;
+   boolevicting;
+
/* BOs who needs a validation */
struct list_headevicted;
 
-- 
2.17.1

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

[PATCH 12/12] drm/amdgpu: make sure to never allocate PDs/PTs for invalidations

[Christian König]

Make sure that we never allocate a page table for an invalidation operation.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 22 +-
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 23729fdd34fb..0e8ab09168fc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1416,15 +1416,15 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_vm_update_params *params,
uint64_t incr, entry_end, pe_start;
struct amdgpu_bo *pt;
 
-   /* make sure that the page tables covering the address range are
-* actually allocated
-*/
-   r = amdgpu_vm_alloc_pts(params->adev, params->vm, ,
-   params->direct);
-   if (r)
-   return r;
-
-   pt = cursor.entry->base.bo;
+   if (flags & AMDGPU_PTE_VALID) {
+   /* make sure that the page tables covering the
+* address range are actually allocated
+*/
+   r = amdgpu_vm_alloc_pts(params->adev, params->vm,
+   , params->direct);
+   if (r)
+   return r;
+   }
 
shift = amdgpu_vm_level_shift(adev, cursor.level);
parent_shift = amdgpu_vm_level_shift(adev, cursor.level - 1);
@@ -1453,6 +1453,10 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_vm_update_params *params,
continue;
}
 
+   pt = cursor.entry->base.bo;
+   if (!pt)
+   return -ENOENT;
+
/* Looks good so far, calculate parameters for the update */
incr = (uint64_t)AMDGPU_GPU_PAGE_SIZE << shift;
mask = amdgpu_vm_entries_mask(adev, cursor.level);
-- 
2.17.1

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

[PATCH 10/12] drm/amdgpu: immedially invalidate PTEs

[Christian König]

When a BO is evicted immedially invalidate the mapped PTEs.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index a03cfbe670c4..6844ba7467a6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2569,6 +2569,7 @@ void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
 struct amdgpu_bo *bo, bool evicted)
 {
struct amdgpu_vm_bo_base *bo_base;
+   int r;
 
/* shadow bo doesn't have bo base, its validation needs its parent */
if (bo->parent && bo->parent->shadow == bo)
@@ -2576,8 +2577,22 @@ void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
 
for (bo_base = bo->vm_bo; bo_base; bo_base = bo_base->next) {
struct amdgpu_vm *vm = bo_base->vm;
+   struct dma_resv *resv = vm->root.base.bo->tbo.base.resv;
+
+   if (bo->tbo.type != ttm_bo_type_kernel) {
+   struct amdgpu_bo_va *bo_va;
+
+   bo_va = container_of(bo_base, struct amdgpu_bo_va,
+base);
+   r = amdgpu_vm_bo_update(adev, bo_va,
+   bo->tbo.base.resv != resv);
+   if (!r) {
+   amdgpu_vm_bo_idle(bo_base);
+   continue;
+   }
+   }
 
-   if (evicted && bo->tbo.base.resv == 
vm->root.base.bo->tbo.base.resv) {
+   if (evicted && bo->tbo.base.resv == resv) {
amdgpu_vm_bo_evicted(bo_base);
continue;
}
-- 
2.17.1

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

[PATCH 01/12] drm/amdgpu: explicitely sync to VM updates v2

[Christian König]

Allows us to reduce the overhead while syncing to fences a bit.

v2: also drop adev parameter from the functions

Signed-off-by: Christian König 
Reviewed-by: Felix Kuehling 
---
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  8 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 19 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c   | 13 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c   |  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c  | 38 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h  |  8 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c   |  2 +-
 7 files changed, 51 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index b6d1958d514f..d8db5ecdf9c1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -358,7 +358,7 @@ static int vm_update_pds(struct amdgpu_vm *vm, struct 
amdgpu_sync *sync)
if (ret)
return ret;
 
-   return amdgpu_sync_fence(NULL, sync, vm->last_update, false);
+   return amdgpu_sync_fence(sync, vm->last_update, false);
 }
 
 static uint64_t get_pte_flags(struct amdgpu_device *adev, struct kgd_mem *mem)
@@ -751,7 +751,7 @@ static int unmap_bo_from_gpuvm(struct amdgpu_device *adev,
 
amdgpu_vm_clear_freed(adev, vm, _va->last_pt_update);
 
-   amdgpu_sync_fence(NULL, sync, bo_va->last_pt_update, false);
+   amdgpu_sync_fence(sync, bo_va->last_pt_update, false);
 
return 0;
 }
@@ -770,7 +770,7 @@ static int update_gpuvm_pte(struct amdgpu_device *adev,
return ret;
}
 
-   return amdgpu_sync_fence(NULL, sync, bo_va->last_pt_update, false);
+   return amdgpu_sync_fence(sync, bo_va->last_pt_update, false);
 }
 
 static int map_bo_to_gpuvm(struct amdgpu_device *adev,
@@ -2045,7 +2045,7 @@ int amdgpu_amdkfd_gpuvm_restore_process_bos(void *info, 
struct dma_fence **ef)
pr_debug("Memory eviction: Validate BOs failed. Try 
again\n");
goto validate_map_fail;
}
-   ret = amdgpu_sync_fence(NULL, _obj, bo->tbo.moving, false);
+   ret = amdgpu_sync_fence(_obj, bo->tbo.moving, false);
if (ret) {
pr_debug("Memory eviction: Sync BO fence failed. Try 
again\n");
goto validate_map_fail;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 9e0c99760367..614919f265b9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -799,29 +799,23 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser 
*p)
if (r)
return r;
 
-   r = amdgpu_sync_fence(adev, >job->sync,
- fpriv->prt_va->last_pt_update, false);
+   r = amdgpu_sync_vm_fence(>job->sync, fpriv->prt_va->last_pt_update);
if (r)
return r;
 
if (amdgpu_mcbp || amdgpu_sriov_vf(adev)) {
-   struct dma_fence *f;
-
bo_va = fpriv->csa_va;
BUG_ON(!bo_va);
r = amdgpu_vm_bo_update(adev, bo_va, false);
if (r)
return r;
 
-   f = bo_va->last_pt_update;
-   r = amdgpu_sync_fence(adev, >job->sync, f, false);
+   r = amdgpu_sync_vm_fence(>job->sync, bo_va->last_pt_update);
if (r)
return r;
}
 
amdgpu_bo_list_for_each_entry(e, p->bo_list) {
-   struct dma_fence *f;
-
/* ignore duplicates */
bo = ttm_to_amdgpu_bo(e->tv.bo);
if (!bo)
@@ -835,8 +829,7 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p)
if (r)
return r;
 
-   f = bo_va->last_pt_update;
-   r = amdgpu_sync_fence(adev, >job->sync, f, false);
+   r = amdgpu_sync_vm_fence(>job->sync, bo_va->last_pt_update);
if (r)
return r;
}
@@ -849,7 +842,7 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p)
if (r)
return r;
 
-   r = amdgpu_sync_fence(adev, >job->sync, vm->last_update, false);
+   r = amdgpu_sync_vm_fence(>job->sync, vm->last_update);
if (r)
return r;
 
@@ -991,7 +984,7 @@ static int amdgpu_cs_process_fence_dep(struct 
amdgpu_cs_parser *p,
dma_fence_put(old);
}
 
-   r = amdgpu_sync_fence(p->adev, >job->sync, fence, true);
+   r = amdgpu_sync_fence(>job->sync, fence, true);
dma_fence_put(fence);
if (r)
return r;
@@ -1013,7 +1006,7 @@ static int amdgpu_syncobj_lookup_and_add_to_sync(struct 
amdgpu_cs_parser *p,

[PATCH 09/12] drm/amdgpu: stop using amdgpu_bo_gpu_offset in the VM backend

[Christian König]

We need to update page tables without any lock held.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
index e7a383134521..4cc7881f438c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
@@ -140,7 +140,7 @@ static void amdgpu_vm_sdma_copy_ptes(struct 
amdgpu_vm_update_params *p,
 
src += p->num_dw_left * 4;
 
-   pe += amdgpu_bo_gpu_offset(bo);
+   pe += amdgpu_gmc_sign_extend(bo->tbo.offset);
trace_amdgpu_vm_copy_ptes(pe, src, count, p->direct);
 
amdgpu_vm_copy_pte(p->adev, ib, pe, src, count);
@@ -167,7 +167,7 @@ static void amdgpu_vm_sdma_set_ptes(struct 
amdgpu_vm_update_params *p,
 {
struct amdgpu_ib *ib = p->job->ibs;
 
-   pe += amdgpu_bo_gpu_offset(bo);
+   pe += amdgpu_gmc_sign_extend(bo->tbo.offset);
trace_amdgpu_vm_set_ptes(pe, addr, count, incr, flags, p->direct);
if (count < 3) {
amdgpu_vm_write_pte(p->adev, ib, pe, addr | flags,
-- 
2.17.1

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

VM changes for invalidating PTEs without holding the reservation lock

[Christian König]

Hi guys,

I'm still narrowing down one problem with the glob lru lock, but that only 
happens in a constructed test case so far.

Please take a look and comment,
Christian.


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

[PATCH 02/12] drm/amdgpu: stop adding VM updates fences to the resv obj

[Christian König]

Don't add the VM update fences to the resv object and remove
the handling to stop implicitely syncing to them.

Ongoing updates prevent page tables from being evicted and we manually
block for all updates to complete before releasing PDs and PTS.

This way we can do updates even without the resv obj locked.

Signed-off-by: Christian König 
Reviewed-by: Felix Kuehling 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c| 10 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 30 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  4 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 11 +---
 4 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
index f1e5fbef54d8..a09b6b9c27d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
@@ -240,13 +240,11 @@ int amdgpu_sync_resv(struct amdgpu_device *adev,
continue;
 
if (amdgpu_sync_same_dev(adev, f)) {
-   /* VM updates are only interesting
-* for other VM updates and moves.
+   /* VM updates only sync with moves but not with user
+* command submissions or KFD evictions fences
 */
-   if ((owner != AMDGPU_FENCE_OWNER_UNDEFINED) &&
-   (fence_owner != AMDGPU_FENCE_OWNER_UNDEFINED) &&
-   ((owner == AMDGPU_FENCE_OWNER_VM) !=
-(fence_owner == AMDGPU_FENCE_OWNER_VM)))
+   if (owner == AMDGPU_FENCE_OWNER_VM &&
+   fence_owner != AMDGPU_FENCE_OWNER_UNDEFINED)
continue;
 
/* Ignore fence from the same owner and explicit one as
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index a22bd57129d1..0d700e8154c4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -562,8 +562,8 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
 {
entry->priority = 0;
entry->tv.bo = >root.base.bo->tbo;
-   /* One for the VM updates, one for TTM and one for the CS job */
-   entry->tv.num_shared = 3;
+   /* One for TTM and one for the CS job */
+   entry->tv.num_shared = 2;
entry->user_pages = NULL;
list_add(>tv.head, validated);
 }
@@ -2522,6 +2522,11 @@ bool amdgpu_vm_evictable(struct amdgpu_bo *bo)
if (!dma_resv_test_signaled_rcu(bo->tbo.base.resv, true))
return false;
 
+   /* Don't evict VM page tables while they are updated */
+   if (!dma_fence_is_signaled(bo_base->vm->last_direct) ||
+   !dma_fence_is_signaled(bo_base->vm->last_delayed))
+   return false;
+
return true;
 }
 
@@ -2687,8 +2692,16 @@ void amdgpu_vm_adjust_size(struct amdgpu_device *adev, 
uint32_t min_vm_size,
  */
 long amdgpu_vm_wait_idle(struct amdgpu_vm *vm, long timeout)
 {
-   return dma_resv_wait_timeout_rcu(vm->root.base.bo->tbo.base.resv,
-  true, true, timeout);
+   timeout = dma_resv_wait_timeout_rcu(vm->root.base.bo->tbo.base.resv,
+   true, true, timeout);
+   if (timeout <= 0)
+   return timeout;
+
+   timeout = dma_fence_wait_timeout(vm->last_direct, true, timeout);
+   if (timeout <= 0)
+   return timeout;
+
+   return dma_fence_wait_timeout(vm->last_delayed, true, timeout);
 }
 
 /**
@@ -2757,6 +2770,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct 
amdgpu_vm *vm,
else
vm->update_funcs = _vm_sdma_funcs;
vm->last_update = NULL;
+   vm->last_direct = dma_fence_get_stub();
+   vm->last_delayed = dma_fence_get_stub();
 
amdgpu_vm_bo_param(adev, vm, adev->vm_manager.root_level, false, );
if (vm_context == AMDGPU_VM_CONTEXT_COMPUTE)
@@ -2807,6 +2822,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct 
amdgpu_vm *vm,
vm->root.base.bo = NULL;
 
 error_free_delayed:
+   dma_fence_put(vm->last_direct);
+   dma_fence_put(vm->last_delayed);
drm_sched_entity_destroy(>delayed);
 
 error_free_direct:
@@ -3007,6 +3024,11 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct 
amdgpu_vm *vm)
vm->pasid = 0;
}
 
+   dma_fence_wait(vm->last_direct, false);
+   dma_fence_put(vm->last_direct);
+   dma_fence_wait(vm->last_delayed, false);
+   dma_fence_put(vm->last_delayed);
+
list_for_each_entry_safe(mapping, tmp, >freed, list) {
if (mapping->flags & AMDGPU_PTE_PRT && prt_fini_needed) {
amdgpu_vm_prt_fini(adev, vm);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h

[PATCH 08/12] drm/amdgpu: rework synchronization of VM updates

[Christian König]

If provided we only sync to the BOs reservation
object and no longer to the root PD.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c|  7 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 34 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  4 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c  | 25 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 15 +++--
 5 files changed, 35 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
index c124f64e7aae..5816df9f8531 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
@@ -240,13 +240,6 @@ int amdgpu_sync_resv(struct amdgpu_device *adev, struct 
amdgpu_sync *sync,
owner != AMDGPU_FENCE_OWNER_UNDEFINED)
continue;
 
-   /* VM updates only sync with moves but not with user
-* command submissions or KFD evictions fences
-*/
-   if (fence_owner != AMDGPU_FENCE_OWNER_UNDEFINED &&
-   owner == AMDGPU_FENCE_OWNER_VM)
-   continue;
-
/* Ignore fences depending on the sync mode */
switch (mode) {
case AMDGPU_SYNC_ALWAYS:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 54430c9f7a27..a03cfbe670c4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -777,7 +777,7 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
params.vm = vm;
params.direct = direct;
 
-   r = vm->update_funcs->prepare(, AMDGPU_FENCE_OWNER_KFD, NULL);
+   r = vm->update_funcs->prepare(, NULL, AMDGPU_SYNC_EXPLICIT);
if (r)
return r;
 
@@ -1273,7 +1273,7 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
params.vm = vm;
params.direct = direct;
 
-   r = vm->update_funcs->prepare(, AMDGPU_FENCE_OWNER_VM, NULL);
+   r = vm->update_funcs->prepare(, NULL, AMDGPU_SYNC_EXPLICIT);
if (r)
return r;
 
@@ -1524,7 +1524,7 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_vm_update_params *params,
  * @adev: amdgpu_device pointer
  * @vm: requested vm
  * @direct: direct submission in a page fault
- * @exclusive: fence we need to sync to
+ * @resv: fences we need to sync to
  * @start: start of mapped range
  * @last: last mapped entry
  * @flags: flags for the entries
@@ -1539,14 +1539,14 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_vm_update_params *params,
  */
 static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
   struct amdgpu_vm *vm, bool direct,
-  struct dma_fence *exclusive,
+  struct dma_resv *resv,
   uint64_t start, uint64_t last,
   uint64_t flags, uint64_t addr,
   dma_addr_t *pages_addr,
   struct dma_fence **fence)
 {
struct amdgpu_vm_update_params params;
-   void *owner = AMDGPU_FENCE_OWNER_VM;
+   enum amdgpu_sync_mode sync_mode;
int r;
 
memset(, 0, sizeof(params));
@@ -1557,7 +1557,9 @@ static int amdgpu_vm_bo_update_mapping(struct 
amdgpu_device *adev,
 
/* sync to everything except eviction fences on unmapping */
if (!(flags & AMDGPU_PTE_VALID))
-   owner = AMDGPU_FENCE_OWNER_KFD;
+   sync_mode = AMDGPU_SYNC_EQ_OWNER;
+   else
+   sync_mode = AMDGPU_SYNC_EXPLICIT;
 
mutex_lock(>eviction_lock);
if (vm->evicting) {
@@ -1565,7 +1567,7 @@ static int amdgpu_vm_bo_update_mapping(struct 
amdgpu_device *adev,
goto error_unlock;
}
 
-   r = vm->update_funcs->prepare(, owner, exclusive);
+   r = vm->update_funcs->prepare(, resv, sync_mode);
if (r)
goto error_unlock;
 
@@ -1584,7 +1586,7 @@ static int amdgpu_vm_bo_update_mapping(struct 
amdgpu_device *adev,
  * amdgpu_vm_bo_split_mapping - split a mapping into smaller chunks
  *
  * @adev: amdgpu_device pointer
- * @exclusive: fence we need to sync to
+ * @resv: fences we need to sync to
  * @pages_addr: DMA addresses to use for mapping
  * @vm: requested vm
  * @mapping: mapped range and flags to use for the update
@@ -1600,7 +1602,7 @@ static int amdgpu_vm_bo_update_mapping(struct 
amdgpu_device *adev,
  * 0 for success, -EINVAL for failure.
  */
 static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
- struct dma_fence *exclusive,
+ struct dma_resv *resv,
  dma_addr_t *pages_addr,
  struct 

[PATCH 06/12] drm/amdgpu: use the VM as job owner

[Christian König]

For HMM we need to rework how VM synchronization works, so instead of the filp 
use VM as job owner.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index c4a8148b9b6f..cf79f30c0af6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -655,6 +655,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
 
 static int amdgpu_cs_sync_rings(struct amdgpu_cs_parser *p)
 {
+   struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
struct amdgpu_bo_list_entry *e;
int r;
 
@@ -662,7 +663,7 @@ static int amdgpu_cs_sync_rings(struct amdgpu_cs_parser *p)
struct amdgpu_bo *bo = ttm_to_amdgpu_bo(e->tv.bo);
struct dma_resv *resv = bo->tbo.base.resv;
 
-   r = amdgpu_sync_resv(p->adev, >job->sync, resv, p->filp,
+   r = amdgpu_sync_resv(p->adev, >job->sync, resv, >vm,
 amdgpu_bo_explicit_sync(bo));
 
if (r)
@@ -1210,7 +1211,7 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
job = p->job;
p->job = NULL;
 
-   r = drm_sched_job_init(>base, entity, p->filp);
+   r = drm_sched_job_init(>base, entity, >vm);
if (r)
goto error_unlock;
 
-- 
2.17.1

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

[PATCH 07/12] drm/amdgpu: rework job synchronization

[Christian König]

For unlocked page table updates we need to be able
to sync to fences of a specific VM.

Signed-off-by: Christian König 
---
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  6 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c|  8 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c|  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c  | 49 ---
 drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h  | 15 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c   |  7 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c   |  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c   |  2 +-
 8 files changed, 59 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index d8db5ecdf9c1..9e7889c28f3e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -848,9 +848,9 @@ static int process_sync_pds_resv(struct amdkfd_process_info 
*process_info,
vm_list_node) {
struct amdgpu_bo *pd = peer_vm->root.base.bo;
 
-   ret = amdgpu_sync_resv(NULL,
-   sync, pd->tbo.base.resv,
-   AMDGPU_FENCE_OWNER_KFD, false);
+   ret = amdgpu_sync_resv(NULL, sync, pd->tbo.base.resv,
+  AMDGPU_SYNC_NE_OWNER,
+  AMDGPU_FENCE_OWNER_KFD);
if (ret)
return ret;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index cf79f30c0af6..d7b5efaa091c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -662,10 +662,12 @@ static int amdgpu_cs_sync_rings(struct amdgpu_cs_parser 
*p)
list_for_each_entry(e, >validated, tv.head) {
struct amdgpu_bo *bo = ttm_to_amdgpu_bo(e->tv.bo);
struct dma_resv *resv = bo->tbo.base.resv;
+   enum amdgpu_sync_mode sync_mode;
 
-   r = amdgpu_sync_resv(p->adev, >job->sync, resv, >vm,
-amdgpu_bo_explicit_sync(bo));
-
+   sync_mode = amdgpu_bo_explicit_sync(bo) ?
+   AMDGPU_SYNC_EXPLICIT : AMDGPU_SYNC_NE_OWNER;
+   r = amdgpu_sync_resv(p->adev, >job->sync, resv, sync_mode,
+>vm);
if (r)
return r;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index ca9f74585890..46c76e2e1281 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -1419,7 +1419,8 @@ int amdgpu_bo_sync_wait(struct amdgpu_bo *bo, void 
*owner, bool intr)
int r;
 
amdgpu_sync_create();
-   amdgpu_sync_resv(adev, , bo->tbo.base.resv, owner, false);
+   amdgpu_sync_resv(adev, , bo->tbo.base.resv,
+AMDGPU_SYNC_NE_OWNER, owner);
r = amdgpu_sync_wait(, intr);
amdgpu_sync_free();
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
index a09b6b9c27d1..c124f64e7aae 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
@@ -202,18 +202,17 @@ int amdgpu_sync_vm_fence(struct amdgpu_sync *sync, struct 
dma_fence *fence)
  *
  * @sync: sync object to add fences from reservation object to
  * @resv: reservation object with embedded fence
- * @explicit_sync: true if we should only sync to the exclusive fence
+ * @mode: how owner affects which fences we sync to
+ * @owner: owner of the planned job submission
  *
  * Sync to the fence
  */
-int amdgpu_sync_resv(struct amdgpu_device *adev,
-struct amdgpu_sync *sync,
-struct dma_resv *resv,
-void *owner, bool explicit_sync)
+int amdgpu_sync_resv(struct amdgpu_device *adev, struct amdgpu_sync *sync,
+struct dma_resv *resv, enum amdgpu_sync_mode mode,
+void *owner)
 {
struct dma_resv_list *flist;
struct dma_fence *f;
-   void *fence_owner;
unsigned i;
int r = 0;
 
@@ -229,6 +228,8 @@ int amdgpu_sync_resv(struct amdgpu_device *adev,
return r;
 
for (i = 0; i < flist->shared_count; ++i) {
+   void *fence_owner;
+
f = rcu_dereference_protected(flist->shared[i],
  dma_resv_held(resv));
/* We only want to trigger KFD eviction fences on
@@ -239,20 +240,34 @@ int amdgpu_sync_resv(struct amdgpu_device *adev,
owner != AMDGPU_FENCE_OWNER_UNDEFINED)
continue;
 
-   if (amdgpu_sync_same_dev(adev, f)) {
-   /* VM updates only sync with moves but 

[PATCH 05/12] drm/amdgpu: explicitly sync VM update to PDs/PTs

[Christian König]

Explicitly sync VM updates to the moving fence in PDs and PTs.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c  | 7 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 5 +
 2 files changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
index 73fec7a0ced5..68b013be3837 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
@@ -86,6 +86,13 @@ static int amdgpu_vm_cpu_update(struct 
amdgpu_vm_update_params *p,
 {
unsigned int i;
uint64_t value;
+   int r;
+
+   if (bo->tbo.moving) {
+   r = dma_fence_wait(bo->tbo.moving, true);
+   if (r)
+   return r;
+   }
 
pe += (unsigned long)amdgpu_bo_kptr(bo);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
index 19b7f80758f1..4bbd8ff778ea 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
@@ -208,6 +208,11 @@ static int amdgpu_vm_sdma_update(struct 
amdgpu_vm_update_params *p,
uint64_t *pte;
int r;
 
+   /* Wait for PD/PT moves to be completed */
+   r = amdgpu_sync_fence(>job->sync, bo->tbo.moving, false);
+   if (r)
+   return r;
+
do {
ndw = p->num_dw_left;
ndw -= p->job->ibs->length_dw;
-- 
2.17.1

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

[PATCH 11/12] drm/amdgpu: drop unnecessary restriction for huge root PDEs

[Christian König]

The root PD can also contain huge PDEs.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 25 +
 1 file changed, 5 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 6844ba7467a6..23729fdd34fb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -94,23 +94,17 @@ struct amdgpu_prt_cb {
 static unsigned amdgpu_vm_level_shift(struct amdgpu_device *adev,
  unsigned level)
 {
-   unsigned shift = 0xff;
-
switch (level) {
case AMDGPU_VM_PDB2:
case AMDGPU_VM_PDB1:
case AMDGPU_VM_PDB0:
-   shift = 9 * (AMDGPU_VM_PDB0 - level) +
+   return 9 * (AMDGPU_VM_PDB0 - level) +
adev->vm_manager.block_size;
-   break;
case AMDGPU_VM_PTB:
-   shift = 0;
-   break;
+   return 0;
default:
-   dev_err(adev->dev, "the level%d isn't supported.\n", level);
+   return ~0;
}
-
-   return shift;
 }
 
 /**
@@ -1432,13 +1426,6 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_vm_update_params *params,
 
pt = cursor.entry->base.bo;
 
-   /* The root level can't be a huge page */
-   if (cursor.level == adev->vm_manager.root_level) {
-   if (!amdgpu_vm_pt_descendant(adev, ))
-   return -ENOENT;
-   continue;
-   }
-
shift = amdgpu_vm_level_shift(adev, cursor.level);
parent_shift = amdgpu_vm_level_shift(adev, cursor.level - 1);
if (adev->asic_type < CHIP_VEGA10 &&
@@ -1457,11 +1444,9 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_vm_update_params *params,
if (!amdgpu_vm_pt_descendant(adev, ))
return -ENOENT;
continue;
-   } else if (frag >= parent_shift &&
-  cursor.level - 1 != adev->vm_manager.root_level) {
+   } else if (frag >= parent_shift) {
/* If the fragment size is even larger than the parent
-* shift we should go up one level and check it again
-* unless one level up is the root level.
+* shift we should go up one level and check it again.
 */
if (!amdgpu_vm_pt_ancestor())
return -ENOENT;
-- 
2.17.1

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

[PATCH 04/12] drm/amdgpu: drop amdgpu_job.owner

[Christian König]

Entirely unused.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  | 1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.h | 1 -
 3 files changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 614919f265b9..c4a8148b9b6f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1233,7 +1233,6 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
goto error_abort;
}
 
-   job->owner = p->filp;
p->fence = dma_fence_get(>base.s_fence->finished);
 
amdgpu_ctx_add_fence(p->ctx, entity, p->fence, );
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index 73328d0c741d..d42be880a236 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -153,7 +153,6 @@ int amdgpu_job_submit(struct amdgpu_job *job, struct 
drm_sched_entity *entity,
if (r)
return r;
 
-   job->owner = owner;
*f = dma_fence_get(>base.s_fence->finished);
amdgpu_job_free_resources(job);
priority = job->base.s_priority;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
index aa0e375062f2..31cb0203 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
@@ -49,7 +49,6 @@ struct amdgpu_job {
uint32_tpreamble_status;
uint32_tpreemption_status;
uint32_tnum_ibs;
-   void*owner;
boolvm_needs_flush;
uint64_tvm_pd_addr;
unsignedvmid;
-- 
2.17.1

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

Re: Warning: check cp_fw_version and update it to realize GRBM requires 1-cycle delay in cp firmware

[Christian König]

Am 06.01.20 um 12:29 schrieb Michel Dänzer:

On 2019-12-26 5:53 p.m., Alex Deucher wrote:

On Thu, Dec 26, 2019 at 5:11 AM Paul Menzel

[   13.446975] [drm] Warning: check cp_fw_version and update it to realize  
GRBM requires 1-cycle delay in cp firmware

Chang, it looks like you added that warning in commit 11c6108934.


drm/amdgpu: add warning for GRBM 1-cycle delay issue in gfx9

It needs to add warning to update firmware in gfx9
in case that firmware is too old to have function to
realize dummy read in cp firmware.

Unfortunately, it looks like you did not even check how the warning is
formatted (needless spaces), so I guess this was totally untested. Also,
what is that warning about, and what is the user supposed to do? I am
unable to find `cp_fw_version` in the source code at all.


The code looks fine.  Not sure why it's rendering funny in your log.
DRM_WARN_ONCE("Warning: check cp_fw_version and update
it to realize \
  GRBM requires 1-cycle delay in cp firmware\n");

Looks like the leading spaces after the backslash are included in the
string. Something like this should be better:

 DRM_WARN_ONCE("Warning: check cp_fw_version and update "
   "GRBM requires 1-cycle delay in cp firmware\n");


Warning and error messages in the kernel should be on a single line if 
possible to allow for searching the kernel code for the string of the 
message.


I suggest to just shorten the message. Especially it is irrelevant why 
the CP firmware is to old, that is debugging info at best. Additional to 
that the "Warning:" is superfluous.


Something like DRM_WARN_ONCE("CP firmware version to old, please 
update!") should perfectly do it.


Regards,
Christian.



(or maybe the intention was to put the second sentence on a new line?)


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

Re: Warning: check cp_fw_version and update it to realize GRBM requires 1-cycle delay in cp firmware

[Michel Dänzer]

On 2019-12-26 5:53 p.m., Alex Deucher wrote:
> On Thu, Dec 26, 2019 at 5:11 AM Paul Menzel
>>
>>> [   13.446975] [drm] Warning: check cp_fw_version and update it to realize  
>>> GRBM requires 1-cycle delay in cp firmware
>>
>> Chang, it looks like you added that warning in commit 11c6108934.
>>
>>> drm/amdgpu: add warning for GRBM 1-cycle delay issue in gfx9
>>>
>>> It needs to add warning to update firmware in gfx9
>>> in case that firmware is too old to have function to
>>> realize dummy read in cp firmware.
>>
>> Unfortunately, it looks like you did not even check how the warning is
>> formatted (needless spaces), so I guess this was totally untested. Also,
>> what is that warning about, and what is the user supposed to do? I am
>> unable to find `cp_fw_version` in the source code at all.
>>
> 
> The code looks fine.  Not sure why it's rendering funny in your log.
>DRM_WARN_ONCE("Warning: check cp_fw_version and update
> it to realize \
>  GRBM requires 1-cycle delay in cp firmware\n");

Looks like the leading spaces after the backslash are included in the
string. Something like this should be better:

DRM_WARN_ONCE("Warning: check cp_fw_version and update "
  "GRBM requires 1-cycle delay in cp firmware\n");

(or maybe the intention was to put the second sentence on a new line?)


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

[PATCH v2] drm/dp_mst: clear time slots for ports invalid

[Wayne Lin]

[Why]
When change the connection status in a MST topology, mst device
which detect the event will send out CONNECTION_STATUS_NOTIFY messgae.

e.g. src-mst-mst-sst => src-mst (unplug) mst-sst

Currently, under the above case of unplugging device, ports which have
been allocated payloads and are no longer in the topology still occupy
time slots and recorded in proposed_vcpi[] of topology manager.

If we don't clean up the proposed_vcpi[], when code flow goes to try to
update payload table by calling drm_dp_update_payload_part1(), we will
fail at checking port validation due to there are ports with proposed
time slots but no longer in the mst topology. As the result of that, we
will also stop updating the DPCD payload table of down stream port.

[How]
While handling the CONNECTION_STATUS_NOTIFY message, add a detection to
see if the event indicates that a device is unplugged to an output port.
If the detection is true, then iterrate over all proposed_vcpi[] to
see whether a port of the proposed_vcpi[] is still in the topology or
not. If the port is invalid, set its num_slots to 0.

Thereafter, when try to update payload table by calling
drm_dp_update_payload_part1(), we can successfully update the DPCD
payload table of down stream port and clear the proposed_vcpi[] to NULL.

Changes since v1:(https://patchwork.kernel.org/patch/11275801/)
* Invert the conditional to reduce the indenting

Reviewed-by: Lyude Paul 
Signed-off-by: Wayne Lin 
Cc: sta...@vger.kernel.org
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 25 -
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 6e10f6235009..e37cd6ec6e36 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2321,7 +2321,7 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch 
*mstb,
 {
struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
struct drm_dp_mst_port *port;
-   int old_ddps, ret;
+   int old_ddps, old_input, ret, i;
u8 new_pdt;
bool dowork = false, create_connector = false;
 
@@ -2352,6 +2352,7 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch 
*mstb,
}
 
old_ddps = port->ddps;
+   old_input = port->input;
port->input = conn_stat->input_port;
port->mcs = conn_stat->message_capability_status;
port->ldps = conn_stat->legacy_device_plug_status;
@@ -2376,6 +2377,28 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch 
*mstb,
dowork = false;
}
 
+   if (!old_input && old_ddps != port->ddps && !port->ddps) {
+   for (i = 0; i < mgr->max_payloads; i++) {
+   struct drm_dp_vcpi *vcpi = mgr->proposed_vcpis[i];
+   struct drm_dp_mst_port *port_validated;
+
+   if (!vcpi)
+   continue;
+
+   port_validated =
+   container_of(vcpi, struct drm_dp_mst_port, 
vcpi);
+   port_validated =
+   drm_dp_mst_topology_get_port_validated(mgr, 
port_validated);
+   if (!port_validated) {
+   mutex_lock(>payload_lock);
+   vcpi->num_slots = 0;
+   mutex_unlock(>payload_lock);
+   } else {
+   drm_dp_mst_topology_put_port(port_validated);
+   }
+   }
+   }
+
if (port->connector)
drm_modeset_unlock(>base.lock);
else if (create_connector)
-- 
2.17.1

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

RE: [PATCH v2] drm/dp_mst: correct the shifting in DP_REMOTE_I2C_READ

[Lin, Wayne]

[AMD Public Use]



> -Original Message-
> From: Lyude Paul 
> Sent: Saturday, January 4, 2020 6:02 AM
> To: Lin, Wayne ; dri-de...@lists.freedesktop.org;
> amd-gfx@lists.freedesktop.org
> Cc: Kazlauskas, Nicholas ; Wentland, Harry
> ; Zuo, Jerry ;
> sta...@vger.kernel.org
> Subject: Re: [PATCH v2] drm/dp_mst: correct the shifting in
> DP_REMOTE_I2C_READ
> 
> Oh! Just a friendly tip, I fixed this before pushing your patch:
> 
> ➜  drm-maint git:(drm-misc-fixes) dim push
> dim: 0b1d54cedbb4 ("drm/dp_mst: correct the shifting in
> DP_REMOTE_I2C_READ"): Fixes: SHA1 needs at least 12 digits:
> dim: ad7f8a1f9ce (drm/helper: add Displayport multi-stream helper (v0.6))
> dim: ERROR: issues in commits detected, aborting
> 
> For the future, we have a set of DRM maintainer tools (they're quite useful
> for people who aren't maintainers though!) that you can use to make sure
> your patch is formatted correctly ahead of time:
> 
> https://drm.pages.freedesktop.org/maintainer-tools/dim.html>
>
> Particularly useful commands:
>  * dim sparse # Checks for trivial code issues, like set but unused variables
>  * dim checkpatch # Checks for code style issues
>  * dim fixes $COMMIT_ID # Adds an appropriately formatted Fixes: tag
>  * dim cite $COMMIT_ID # Adds an appropriately formatted Reference: tag
>
Really appreciate for your time!
I will have a look on this. Thanks a lot.
 
> On Fri, 2020-01-03 at 13:50 +0800, Wayne Lin wrote:
> > [Why]
> > According to DP spec, it should shift left 4 digits for NO_STOP_BIT in
> > REMOTE_I2C_READ message. Not 5 digits.
> >
> > In current code, NO_STOP_BIT is always set to zero which means I2C
> > master is always generating a I2C stop at the end of each I2C write
> > transaction while handling REMOTE_I2C_READ sideband message. This
> > issue might have the generated I2C signal not meeting the requirement.
> > Take random read in I2C for instance, I2C master should generate a
> > repeat start to start to read data after writing the read address.
> > This issue will cause the I2C master to generate a stop-start rather
> > than a re-start which is not expected in I2C random read.
> >
> > [How]
> > Correct the shifting value of NO_STOP_BIT for DP_REMOTE_I2C_READ case
> > in drm_dp_encode_sideband_req().
> >
> > Changes since
> >
> v1:(https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> >
> patchwork.kernel.org%2Fpatch%2F11312667%2Fdata=02%7C01%7Cw
> ayne.li
> >
> n%40amd.com%7Ce4299c1721bd4bcb486708d790989148%7C3dd8961fe4884e
> 608e11a
> >
> 82d994e183d%7C0%7C0%7C637136857271065549sdata=eoxiRojffBepY
> YoQbjp
> > j8b6R%2BkwaOQWBoU%2Fu8lY5gIQ%3Dreserved=0)
> > * Add more descriptions in commit and cc to stable
> >
> > Fixes: ad7f8a1f9ce (drm/helper: add Displayport multi-stream helper
> > (v0.6))
> > Reviewed-by: Harry Wentland 
> > Signed-off-by: Wayne Lin 
> > Cc: sta...@vger.kernel.org
> > ---
> >  drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 1cf5f8b8bbb8..9d24c98bece1 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -393,7 +393,7 @@ drm_dp_encode_sideband_req(const struct
> > drm_dp_sideband_msg_req_body *req,
> > memcpy([idx], req-
> > >u.i2c_read.transactions[i].bytes, req-
> > >u.i2c_read.transactions[i].num_bytes);
> > idx += req->u.i2c_read.transactions[i].num_bytes;
> >
> > -   buf[idx] = (req-
> > >u.i2c_read.transactions[i].no_stop_bit & 0x1) << 5;
> > +   buf[idx] = (req-
> > >u.i2c_read.transactions[i].no_stop_bit & 0x1) << 4;
> > buf[idx] |= (req-
> > >u.i2c_read.transactions[i].i2c_transaction_delay & 0xf);
> > idx++;
> > }
> --
> Cheers,
>   Lyude Paul
--
Best Regards,
Wayne Lin
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

回覆: [PATCH] drm/dp_mst: clear time slots for ports invalid

[Lin, Wayne]

[AMD Public Use]



> -原始郵件-
> 寄件者: Lyude Paul 
> 寄件日期: Saturday, January 4, 2020 7:34 AM
> 收件者: Lin, Wayne ; dri-
> de...@lists.freedesktop.org; amd-gfx@lists.freedesktop.org
> 副本: Kazlauskas, Nicholas ; Wentland,
> Harry ; Zuo, Jerry ;
> sta...@vger.kernel.org
> 主旨: Re: [PATCH] drm/dp_mst: clear time slots for ports invalid
> 
> On Wed, 2019-12-25 at 06:45 +, Lin, Wayne wrote:
> > > -Original Message-
> > > From: Lyude Paul 
> > > Sent: Saturday, December 21, 2019 8:12 AM
> > > To: Lin, Wayne ; dri-de...@lists.freedesktop.org;
> > > amd-gfx@lists.freedesktop.org
> > > Cc: Kazlauskas, Nicholas ; Wentland,
> > > Harry ; Zuo, Jerry ;
> > > sta...@vger.kernel.org
> > > Subject: Re: [PATCH] drm/dp_mst: clear time slots for ports invalid
> > >
> > > Mhh-I think I understand the problem you're trying to solve here but
> > > I think this solution might be a bit overkill. When I did the rework
> > > of topology references for ports, I made it so that we can guarantee
> > > memory access to a port without it needing to be a valid part of the
> > > topology. As well, all parents of the port are guaranteed to be
> > > accessible for as long as the child is. Take a look at:
> > >
> > >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2F01
> > > .org%
> > > 2Flinuxgraphics%2Fgfx-docs%2Fdrm%2Fgpu%2Fdrm-kms-
> helpers.html%23refc
> > > o
> > > unt-relationships-in-a-
> topologydata=02%7C01%7Cwayne.lin%40amd.c
> > > o
> m%7C722655b546c049dc081908d785aa6758%7C3dd8961fe4884e608e11a82d
> > >
> 994e183d%7C0%7C0%7C637124839257213115sdata=Ctha3ja8kleeFOp
> > > PpA7EwDV1is81RAMsjqd1P6463ak%3Dreserved=0
> > >
> > > It's also worth noting that because of this there's a lot of
> > > get_port_validated()/put_port_validated() calls in the MST helpers
> > > that are now bogus and need to be removed once I get a chance. For
> > > new code we should limit the use of topology references to sections
> > > of code where we need a guarantee that resources on the port/branch
> > > (such as a drm connector, dp aux port, etc.) won't go away for as
> > > long as we need to use them.
> > >
> > > Do you think we could change this patch so instead of removing it
> > > from the proposed payloads on the CONNECTION_STATUS_NOTIFY, we
> keep
> > > the port's memory allocation around until it's been removed from the
> > > proposed payloads table and clean it up there on the next payload
> > > update?
> > >
> > Really appreciate for your time and comments in detail.
> >
> > In this patch, I wanted to just set the proposed_vcpi->num_slots to 0
> > for those ports which are no longer in the topology due to there is no
> > need to allocate time slots for these port. And expect those vcpi will
> > be updated during next update of payload ID table by
> > drm_dp_update_payload_part1().
> >
> > I tried to use drm_dp_mst_topology_get_port_validated() as a helper to
> > decide whether a port is in the topology or not. Use this function to
> > iterate over all ports that all proposed_vcpi[] drive to. If one port
> > is not in the topology, set the num_slots of the proposed_vcpi for
> > this port to 0. With num_slots as 0, these proposed_vcpi will be clean
> > up in next payload table update by drm_dp_update_payload_part1(). If a
> > port is still in the topology, then release the reference count which
> > was acquired previously from
> > drm_dp_mst_topology_get_port_validated() and do nothing.
> >
> > I didn't mean to kill invalid ports on receiving
> CONNECTION_STATUS_NOTIFY.
> > Sorry if I misuse or misunderstand something here?
> 
> Ahh, it seems I made the mistake here then because from your explanation
> you're using the API exactly as intended :). All of this has me wondering if
> some day we should try to get rid of the payload tracking we have and move
> it into atomic. But, that's a problem for another day.
> 
> Anyway-one small change below:
> 
> >
> > > On Fri, 2019-12-06 at 16:39 +0800, Wayne Lin wrote:
> > > > [Why]
> > > > When change the connection status in a MST topology, mst device
> > > > which detect the event will send out CONNECTION_STATUS_NOTIFY
> messgae.
> > > >
> > > > e.g. src-mst-mst-sst => src-mst (unplug) mst-sst
> > > >
> > > > Currently, under the above case of unplugging device, ports which
> > > > have been allocated payloads and are no longer in the topology
> > > > still occupy time slots and recorded in proposed_vcpi[] of topology
> manager.
> > > >
> > > > If we don't clean up the proposed_vcpi[], when code flow goes to
> > > > try to update payload table by calling
> > > > drm_dp_update_payload_part1(), we will fail at checking port
> > > > validation due to there are ports with proposed time slots but no
> > > > longer in the mst topology. As the result of that, we will also
> > > > stop updating the DPCD payload table of down stream
> > > port.
> > > > [How]
> > > > While handling the CONNECTION_STATUS_NOTIFY message, add a
> > > > detection to see if the event indicates that a device