[PATCH] drm/admgpu: check HDMI HPD status after ddc probe

2020-05-08 Thread Binbin Zhou 
Now, we check the presence of the EDID to determine if there is a monitor
present.

DVI-I connectors have both analog and digital encoders and the HPD pin
is only reliable on the digital part.

But when I pull out the Radeon HD8570's HDMI connector, the HDMI status
in system is still perform connected.

asd@asd-PC:~$ cat /sys/class/drm/card0-HDMI-A-1/status 
connected

At this moment, if I want to read the EDID by /dev/i2c-X with I2C
driver, there is no EDID can be read.

Dmesg witha drm.debug=0x6, we can find the following message:

[drm:drm_helper_hpd_irq_event] [CONNECTOR:41:HDMI-A-1] status
updated from connected to connected

Based on the appearance, I thought to check the HPD status again, because
the HPD status is perform disconnected, after amdgpu_display_ddc_probe().
If the amdgpu_display_hpd_sense() return false, I think the HDMI connector
status is undefined, and just return disconnected simply.

I'm not sure if it happened to other AMD cards.

Signed-off-by: Binbin Zhou 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index f355d9a752d2..ee657db9a228 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -973,7 +973,7 @@ amdgpu_connector_dvi_detect(struct drm_connector 
*connector, bool force)
const struct drm_encoder_helper_funcs *encoder_funcs;
int r;
enum drm_connector_status ret = connector_status_disconnected;
-   bool dret = false, broken_edid = false;
+   bool dret = false, broken_edid = false, undefined_flag = false;
 
if (!drm_kms_helper_is_poll_worker()) {
r = pm_runtime_get_sync(connector->dev->dev);
@@ -988,7 +988,12 @@ amdgpu_connector_dvi_detect(struct drm_connector 
*connector, bool force)
 
if (amdgpu_connector->ddc_bus)
dret = amdgpu_display_ddc_probe(amdgpu_connector, false);
-   if (dret) {
+
+   /* Check the HDMI HPD pin status again */
+   if (!amdgpu_display_hpd_sense(adev, amdgpu_connector->hpd.hpd))
+   undefined_flag = true;
+
+   if (dret && !undefined_flag) {
amdgpu_connector->detected_by_load = false;
amdgpu_connector_free_edid(connector);
amdgpu_connector_get_edid(connector);
-- 
2.17.1

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

[PATCH] drm/amd/powerplay: report correct AC/DC event based on ctxid V2

2020-05-08 Thread Evan Quan 
'ctxid' is used to distinguish different events raised from SMC.
0x3 and 0x4 are for AC and DC power mode.

V2: update the way to retrieve the ctxid and change the log level
to debug

Change-Id: I7dbcb053fe9cea7c70e0a33afc2115b3745f6186
Signed-off-by: Evan Quan 
Reviewed-by: Alex Deucher 
---
 drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 19 +--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c 
b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
index ab727f33b8d9..5792b224b0c3 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
@@ -1522,6 +1522,11 @@ static int smu_v11_0_irq_process(struct amdgpu_device 
*adev,
 {
uint32_t client_id = entry->client_id;
uint32_t src_id = entry->src_id;
+   /*
+* ctxid is used to distinguish different
+* events for SMCToHost interrupt.
+*/
+   uint32_t ctxid = entry->src_data[0];
 
if (client_id == SOC15_IH_CLIENTID_THM) {
switch (src_id) {
@@ -1547,8 +1552,18 @@ static int smu_v11_0_irq_process(struct amdgpu_device 
*adev,
 
}
} else if (client_id == SOC15_IH_CLIENTID_MP1) {
-   if (src_id == 0xfe)
-   smu_v11_0_ack_ac_dc_interrupt(>smu);
+   if (src_id == 0xfe) {
+   switch (ctxid) {
+   case 0x3:
+   dev_dbg(adev->dev, "Switched to AC mode!\n");
+   smu_v11_0_ack_ac_dc_interrupt(>smu);
+   break;
+   case 0x4:
+   dev_dbg(adev->dev, "Switched to DC mode!\n");
+   smu_v11_0_ack_ac_dc_interrupt(>smu);
+   break;
+   }
+   }
}
 
return 0;
-- 
2.26.2

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

RE: [PATCH 1/2] drm/amdgpu: decode the ctxid for SMC to host IH

2020-05-08 Thread Quan, Evan 
[AMD Official Use Only - Internal Distribution Only]

OK. Reasonable. Will update the patches.

BR,
Evan
-Original Message-
From: Christian König 
Sent: Friday, May 8, 2020 6:53 PM
To: Quan, Evan ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander 
Subject: Re: [PATCH 1/2] drm/amdgpu: decode the ctxid for SMC to host IH

Am 08.05.20 um 12:02 schrieb Evan Quan:
> Driver needs that to tell the different events raised from SMC.

NAK, the ctxid is filled in by the SMC and not by the IH. We only decode fields 
here which are filled in by the IH.

Just use src_data[0] when you need some SMC specific bits.

Christian.

>
> Change-Id: I0e44e22527182fbb45a2db4fc3b1cd73fb17ba33
> Signed-off-by: Evan Quan 
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h | 1 +
>   drivers/gpu/drm/amd/amdgpu/navi10_ih.c  | 1 +
>   drivers/gpu/drm/amd/amdgpu/vega10_ih.c  | 1 +
>   3 files changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
> index 7b1762b1c595..965875b8bf93 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
> @@ -55,6 +55,7 @@ struct amdgpu_iv_entry {
>   unsigned timestamp_src;
>   unsigned pasid;
>   unsigned pasid_src;
> +unsigned ctxid; /* for SMC to Host interrupt */
>   unsigned src_data[AMDGPU_IRQ_SRC_DATA_MAX_SIZE_DW];
>   const uint32_t *iv_entry;
>   };
> diff --git a/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
> b/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
> index f97857ed3c7e..932dc3eabbe2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
> +++ b/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
> @@ -470,6 +470,7 @@ static void navi10_ih_decode_iv(struct amdgpu_device 
> *adev,
>   entry->timestamp_src = dw[2] >> 31;
>   entry->pasid = dw[3] & 0x;
>   entry->pasid_src = dw[3] >> 31;
> +entry->ctxid = dw[4];
>   entry->src_data[0] = dw[4];
>   entry->src_data[1] = dw[5];
>   entry->src_data[2] = dw[6];
> diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
> b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
> index 407c6093c2ec..e46d1f9f62ca 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
> @@ -460,6 +460,7 @@ static void vega10_ih_decode_iv(struct amdgpu_device 
> *adev,
>   entry->timestamp_src = dw[2] >> 31;
>   entry->pasid = dw[3] & 0x;
>   entry->pasid_src = dw[3] >> 31;
> +entry->ctxid = dw[4];
>   entry->src_data[0] = dw[4];
>   entry->src_data[1] = dw[5];
>   entry->src_data[2] = dw[6];

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

Re: [PATCH] drm/amd/display: Fix vblank and pageflip event handling for FreeSync

2020-05-08 Thread Mario Kleiner 
On Thu, May 7, 2020 at 7:35 PM Kazlauskas, Nicholas <
nicholas.kazlaus...@amd.com> wrote:

> It applies on top of Alex's amd-staging-drm-next-branch.
>
> It is essentially just a revert to the old behavior with
> acrtc_state->active_planes == 0 special case you added on top and some
> small refactoring.
>
> The only remaining bits that are kind of questionable is the
> VUPDATE_NO_LOCK vs VUPDATE bit again along with some of the locking around
> where we check if FreeSync is active or not.
>
> I also don't think that VSTARTUP is the correct place to be performing the
> update for the registers since the offset between VSTARTUP and VUPDATE can
> be small enough such that the programming won't finish in time for some
> timings. We should be doing it on line 0 instead.
>
> All these issues existed before this patch series at least though.
>
> Regards,
> Nicholas Kazlauskas
>
>
Ok, thanks. Tested it on a Samsung monitor with 48 Hz - 144 Hz range on a
Raven Ridge gpu. It worked with and without your patch, both with my tests,
and with the that VRRTester application from GitHub, so i guess the problem
is highly dependent on small timing differences in game execution, vblank
durations, etc.

So fwif here's my

Reviewed-and-Tested-by: Mario Kleiner 

-mario

On 2020-05-07 12:58 p.m., Mario Kleiner wrote:
>
> Looking over it now, will do some testing. Alex amdgpu-drm-next branch
> would be the best to test this?
>
> It looks like a revert of the whole vstartup series, except for that one
> if(acrtc_state->active_planes == 0)  special case, so i expect it should be
> fine?
>
> thanks,
> -mario
>
>
> On Thu, May 7, 2020 at 5:56 PM Leo Li  wrote:
>
>>
>>
>> On 2020-05-06 3:47 p.m., Nicholas Kazlauskas wrote:
>> > [Why]
>> > We're the drm vblank event a frame too early in the case where the
>> ^sending
>>
>> Thanks for catching this!
>>
>> Reviewed-by: Leo Li 
>>
>> > pageflip happens close to VUPDATE and ends up blocking the signal.
>> >
>> > The implementation in DM was previously correct *before* we started
>> > sending vblank events from VSTARTUP unconditionally to handle cases
>> > where HUBP was off, OTG was ON and userspace was still requesting some
>> > DRM planes enabled. As part of that patch series we dropped VUPDATE
>> > since it was deemed close enough to VSTARTUP, but there's a key
>> > difference betweeen VSTARTUP and VUPDATE - the VUPDATE signal can be
>> > blocked if we're holding the pipe lock >
>> > There was a fix recently to revert the unconditional behavior for the
>> > DCN VSTARTUP vblank event since it was sending the pageflip event on
>> > the wrong frame - once again, due to blocking VUPDATE and having the
>> > address start scanning out two frames later.
>> >
>> > The problem with this fix is it didn't update the logic that calls
>> > drm_crtc_handle_vblank(), so the timestamps are totally bogus now.
>> >
>> > [How]
>> > Essentially reverts most of the original VSTARTUP series but retains
>> > the behavior to send back events when active planes == 0.
>> >
>> > Some refactoring/cleanup was done to not have duplicated code in both
>> > the handlers.
>> >
>> > Fixes: 16f17eda8bad ("drm/amd/display: Send vblank and user events at
>> vsartup for DCN")
>> > Fixes: 3a2ce8d66a4b ("drm/amd/display: Disable VUpdate interrupt for
>> DCN hardware")
>> > Fixes: 2b5aed9ac3f7 ("drm/amd/display: Fix pageflip event race
>> condition for DCN.")
>> >
>> > Cc: Leo Li 
>> > Cc: Mario Kleiner 
>> > Signed-off-by: Nicholas Kazlauskas 
>> > ---
>> >   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 137 +++---
>> >   1 file changed, 55 insertions(+), 82 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 59f1d4a94f12..30ce28f7c444 100644
>> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> > @@ -441,7 +441,7 @@ static void dm_vupdate_high_irq(void
>> *interrupt_params)
>> >
>> >   /**
>> >* dm_crtc_high_irq() - Handles CRTC interrupt
>> > - * @interrupt_params: ignored
>> > + * @interrupt_params: used for determining the CRTC instance
>> >*
>> >* Handles the CRTC/VSYNC interrupt by notfying DRM's VBLANK
>> >* event handler.
>> > @@ -455,70 +455,6 @@ static void dm_crtc_high_irq(void
>> *interrupt_params)
>> >   unsigned long flags;
>> >
>> >   acrtc = get_crtc_by_otg_inst(adev, irq_params->irq_src -
>> IRQ_TYPE_VBLANK);
>> > -
>> > - if (acrtc) {
>> > - acrtc_state = to_dm_crtc_state(acrtc->base.state);
>> > -
>> > - DRM_DEBUG_VBL("crtc:%d, vupdate-vrr:%d\n",
>> > -   acrtc->crtc_id,
>> > -   amdgpu_dm_vrr_active(acrtc_state));
>> > -
>> > - /* Core vblank handling at start of front-porch is only
>> possible
>> > -  * in non-vrr mode, as only there vblank timestamping
>> will give
>> > -  *

Re: [PATCH 2/2] drm/amd/powerplay: report correct AC/DC event based on ctxid

2020-05-08 Thread Alex Deucher 
On Fri, May 8, 2020 at 6:03 AM Evan Quan  wrote:
>
> 'ctxid' is used to distinguish different events raised from SMC.
> 0x3 and 0x4 are for AC and DC power mode.
>
> Change-Id: I7dbcb053fe9cea7c70e0a33afc2115b3745f6186
> Signed-off-by: Evan Quan 
> ---
>  drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 15 +--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c 
> b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> index ab727f33b8d9..dcd3a54d18a3 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> @@ -1522,6 +1522,7 @@ static int smu_v11_0_irq_process(struct amdgpu_device 
> *adev,
>  {
> uint32_t client_id = entry->client_id;
> uint32_t src_id = entry->src_id;
> +   uint32_t ctxid = entry->ctxid;

As noted by Christian, just use the entry->src_data[0] directly.

>
> if (client_id == SOC15_IH_CLIENTID_THM) {
> switch (src_id) {
> @@ -1547,8 +1548,18 @@ static int smu_v11_0_irq_process(struct amdgpu_device 
> *adev,
>
> }
> } else if (client_id == SOC15_IH_CLIENTID_MP1) {
> -   if (src_id == 0xfe)
> -   smu_v11_0_ack_ac_dc_interrupt(>smu);
> +   if (src_id == 0xfe) {
> +   switch (ctxid) {
> +   case 0x3:
> +   dev_info(adev->dev, "Switched to AC mode!\n");
> +   smu_v11_0_ack_ac_dc_interrupt(>smu);
> +   break;
> +   case 0x4:
> +   dev_info(adev->dev, "Switched to DC mode!\n");

Might want to make these dev_debug so we don't spam the logs.  With
those fixed, patch is:
Reviewed-by: Alex Deucher 

> +   smu_v11_0_ack_ac_dc_interrupt(>smu);
> +   break;
> +   }
> +   }
> }
>
> return 0;
> --
> 2.26.2
>
> ___
> 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/amd/display: Fix pageflip event race condition for DCN. (v2)

2020-05-08 Thread Matt Coffin 
Just to follow up on this, I decided not to file the report, since
Nick's latest patch that "Fixes:" this has fixed my issue. Thanks for
the good work on that one Nick.

Might want to make sure that makes it in to the 5.7 fixes :)

Thanks again guys.

~Matt

On 5/5/20 11:59 AM, Kazlauskas, Nicholas wrote:
> Can you file a full bug report on the gitlab tracker?
> 
> FreeSync is still working on my Navi setups with this patch applied, and
> this patch is essentially just a revert of another patch already (where
> FreeSync worked before).
> 
> I can understand the v2 of this series causing issues, but the v1
> shouldn't be - so I'd like to understand more about the setup where this
> is causing issues - ASIC, OS, compositor, displays, dmesg log, X log, etc.
> 
> Regards,
> Nicholas Kazlauskas
> 
> On 2020-05-05 1:03 p.m., Alex Deucher wrote:
>> Mario or Nick any thoughts?
>>
>> Alex
>>
>> On Mon, May 4, 2020 at 1:35 PM Matt Coffin  wrote:
>>>
>>> Hey guys,
>>>
>>> This is still an issue for me, and I'm still having to run a patch to
>>> revert this as of 5.7-rc4. To avoid breaking a lot of people's Navi
>>> setups in 5.7, is there any news on this? Has anyone else at the very
>>> least been able to reproduce the problem?
>>>
>>> It happens for me in every single program that mesa allows to utilize
>>> variable refresh rates, and reverting it "fixes" the issue.
>>>
>>> Cheers, and sorry for the extra email, just making sure this is still on
>>> someone's radar,
>>> Matt
>>>
>>> On 4/14/20 5:32 PM, Matt Coffin wrote:
 Hey everyone,

 This patch broke variable refresh rate in games (all that I've tried so
 far... Project CARS 2, DiRT Rally 2.0, Assetto Corsa Competizione) as
 well as a simple freesync tester application.

 FreeSync tester I've been using: https://github.com/Nixola/VRRTest

 I'm not at all familiar with the page flipping code, so it would
 take me
 a long time to find the *right* way to fix it, but does someone else
 see
 why it would do that?

 The symptom is that the refresh rate of the display constantly bounces
 between the two ends of the FreeSync range (for me 40 -> 144), and the
 game stutters like a madman.

 Any help on where to start, ideas on how to fix it (other than just
 revert this commit, which I've done in the interim), or alternative
 patches would be appreciated.

 Thanks in advance for the work/help,
 Matt

 On 3/13/20 8:42 AM, Michel Dänzer wrote:
> On 2020-03-13 1:35 p.m., Kazlauskas, Nicholas wrote:
>> On 2020-03-12 10:32 a.m., Alex Deucher wrote:
>>> On Thu, Mar 5, 2020 at 4:21 PM Mario Kleiner
>>>  wrote:

 Commit '16f17eda8bad ("drm/amd/display: Send vblank and user
 events at vsartup for DCN")' introduces a new way of pageflip
 completion handling for DCN, and some trouble.

 The current implementation introduces a race condition, which
 can cause pageflip completion events to be sent out one vblank
 too early, thereby confusing userspace and causing flicker:

 prepare_flip_isr():

 1. Pageflip programming takes the ddev->event_lock.
 2. Sets acrtc->pflip_status == AMDGPU_FLIP_SUBMITTED
 3. Releases ddev->event_lock.

 --> Deadline for surface address regs double-buffering passes on
   target pipe.

 4. dc_commit_updates_for_stream() MMIO programs the new pageflip
  into hw, but too late for current vblank.

 => pflip_status == AMDGPU_FLIP_SUBMITTED, but flip won't complete
  in current vblank due to missing the double-buffering deadline
  by a tiny bit.

 5. VSTARTUP trigger point in vblank is reached, VSTARTUP irq fires,
  dm_dcn_crtc_high_irq() gets called.

 6. Detects pflip_status == AMDGPU_FLIP_SUBMITTED and assumes the
  pageflip has been completed/will complete in this vblank and
  sends out pageflip completion event to userspace and resets
  pflip_status = AMDGPU_FLIP_NONE.

 => Flip completion event sent out one vblank too early.

 This behaviour has been observed during my testing with measurement
 hardware a couple of time.

 The commit message says that the extra flip event code was added to
 dm_dcn_crtc_high_irq() to prevent missing to send out pageflip
 events
 in case the pflip irq doesn't fire, because the "DCH HUBP"
 component
 is clock gated and doesn't fire pflip irqs in that state. Also that
 this clock gating may happen if no planes are active. According to
 Nicholas, the clock gating can also happen if psr is active, and
 the
 gating is controlled independently by the hardware, so difficult to
 detect if and when

RE: [PATCH v2] drm/amd/amdgpu: cleanup coding style a bit

2020-05-08 Thread Ruhl, Michael J 
>-Original Message-
>From: dri-devel  On Behalf Of
>Bernard Zhao
>Sent: Thursday, May 7, 2020 5:13 AM
>To: Alex Deucher ; Christian König
>; David (ChunMing) Zhou
>; David Airlie ; Daniel Vetter
>; Tom St Denis ; Sam Ravnborg
>; Ori Messinger ; Bernard
>Zhao ; amd-gfx@lists.freedesktop.org; dri-
>de...@lists.freedesktop.org; linux-ker...@vger.kernel.org
>Cc: opensource.ker...@vivo.com
>Subject: [PATCH v2] drm/amd/amdgpu: cleanup coding style a bit
>
>There is DEVICE_ATTR mechanism in separate attribute define.
>So this change is to use attr array, also use
>sysfs_create_files in init function & sysfs_remove_files in
>fini function.
>This maybe make the code a bit readable.
>
>Signed-off-by: Bernard Zhao 
>
>Changes since V1:
>*Use DEVICE_ATTR mechanism
>
>Link for V1:
>*https://lore.kernel.org/patchwork/patch/1228076/
>---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 43 ++-
>-
> 1 file changed, 13 insertions(+), 30 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>index 82a3299e53c0..57bbc70662ff 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>@@ -148,6 +148,15 @@ static DEVICE_ATTR(mem_info_vis_vram_used,
>S_IRUGO,
> static DEVICE_ATTR(mem_info_vram_vendor, S_IRUGO,
>  amdgpu_mem_info_vram_vendor, NULL);
>
>+static struct attribute *amdgpu_vram_mgr_attributes[] = {
>+  _attr_mem_info_vram_total.attr,
>+  _attr_mem_info_vis_vram_total.attr,
>+  _attr_mem_info_vram_used.attr,
>+  _attr_mem_info_vis_vram_used.attr,
>+  _attr_mem_info_vram_vendor.attr,
>+  NULL
>+};
>+
> /**
>  * amdgpu_vram_mgr_init - init VRAM manager and DRM MM
>  *
>@@ -172,31 +181,9 @@ static int amdgpu_vram_mgr_init(struct
>ttm_mem_type_manager *man,
>   man->priv = mgr;
>
>   /* Add the two VRAM-related sysfs files */
>-  ret = device_create_file(adev->dev,
>_attr_mem_info_vram_total);
>-  if (ret) {
>-  DRM_ERROR("Failed to create device file
>mem_info_vram_total\n");
>-  return ret;
>-  }
>-  ret = device_create_file(adev->dev,
>_attr_mem_info_vis_vram_total);
>-  if (ret) {
>-  DRM_ERROR("Failed to create device file
>mem_info_vis_vram_total\n");
>-  return ret;
>-  }
>-  ret = device_create_file(adev->dev,
>_attr_mem_info_vram_used);
>-  if (ret) {
>-  DRM_ERROR("Failed to create device file
>mem_info_vram_used\n");
>-  return ret;
>-  }
>-  ret = device_create_file(adev->dev,
>_attr_mem_info_vis_vram_used);
>-  if (ret) {
>-  DRM_ERROR("Failed to create device file
>mem_info_vis_vram_used\n");
>-  return ret;
>-  }
>-  ret = device_create_file(adev->dev,
>_attr_mem_info_vram_vendor);
>-  if (ret) {
>-  DRM_ERROR("Failed to create device file
>mem_info_vram_vendor\n");
>-  return ret;
>-  }
>+  ret = sysfs_create_files(>dev->kobj,
>amdgpu_vram_mgr_attributes);
>+  if (ret)
>+  DRM_ERROR("Failed to register sysfs\n");

This looks good to me.

I think that there is a new error macro (drm_err?) that you might
want to use instead of DRM_ERROR().

Otherwise:

Acked-by: Michael J. Ruhl 

m

>
>   return 0;
> }
>@@ -219,11 +206,7 @@ static int amdgpu_vram_mgr_fini(struct
>ttm_mem_type_manager *man)
>   spin_unlock(>lock);
>   kfree(mgr);
>   man->priv = NULL;
>-  device_remove_file(adev->dev, _attr_mem_info_vram_total);
>-  device_remove_file(adev->dev,
>_attr_mem_info_vis_vram_total);
>-  device_remove_file(adev->dev, _attr_mem_info_vram_used);
>-  device_remove_file(adev->dev,
>_attr_mem_info_vis_vram_used);
>-  device_remove_file(adev->dev,
>_attr_mem_info_vram_vendor);
>+  sysfs_remove_files(>dev->kobj,
>amdgpu_vram_mgr_attributes);
>   return 0;
> }
>
>--
>2.26.2
>
>___
>dri-devel mailing list
>dri-de...@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 06/36] drm/amdgpu: use the unlocked drm_gem_object_put

2020-05-08 Thread Sam Ravnborg 
On Fri, May 08, 2020 at 10:55:42AM +0100, Emil Velikov wrote:
> On Fri, 8 May 2020 at 09:16, Christian König  wrote:
> >
> > Am 07.05.20 um 20:03 schrieb Sam Ravnborg:
> > > Hi Emil.
> > >
> > > On Thu, May 07, 2020 at 04:07:52PM +0100, Emil Velikov wrote:
> > >> From: Emil Velikov 
> > >>
> > >> The driver does not hold struct_mutex, thus using the locked version of
> > >> the helper is incorrect.
> > >>
> > >> Cc: Alex Deucher 
> > >> Cc: Christian König 
> > >> Cc: amd-gfx@lists.freedesktop.org
> > >> Fixes: a39414716ca0 ("drm/amdgpu: add independent DMA-buf import v9"):
> > >> Signed-off-by: Emil Velikov 
> > >> ---
> > >>   drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 2 +-
> > >>   1 file changed, 1 insertion(+), 1 deletion(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c 
> > >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> > >> index 43d8ed7dbd00..652c57a3b847 100644
> > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> > >> @@ -587,7 +587,7 @@ struct drm_gem_object 
> > >> *amdgpu_gem_prime_import(struct drm_device *dev,
> > >>  attach = dma_buf_dynamic_attach(dma_buf, dev->dev,
> > >>  _dma_buf_attach_ops, obj);
> > >>  if (IS_ERR(attach)) {
> > >> -drm_gem_object_put(obj);
> > >> +drm_gem_object_put_unlocked(obj);
> > >>  return ERR_CAST(attach);
> > >>  }
> > > Likewise previous patch.
> > > Drop this as the patch is correct after the rename a few pathces later.
> >
> > Well this is a bug fix in the error path and should probably be back
> > ported, so having a separate patch is certainly a good idea.
> >
> Precisely the goal here. The fixes tag should allow Greg and team to
> pick/port it where applicable.
I got it now... Thanks for spelling it out for my dense mind.

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

Re: [PATCH 1/2] drm/amdgpu: decode the ctxid for SMC to host IH

2020-05-08 Thread Christian König 

Am 08.05.20 um 12:02 schrieb Evan Quan:

Driver needs that to tell the different events raised from SMC.


NAK, the ctxid is filled in by the SMC and not by the IH. We only decode 
fields here which are filled in by the IH.


Just use src_data[0] when you need some SMC specific bits.

Christian.



Change-Id: I0e44e22527182fbb45a2db4fc3b1cd73fb17ba33
Signed-off-by: Evan Quan 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h | 1 +
  drivers/gpu/drm/amd/amdgpu/navi10_ih.c  | 1 +
  drivers/gpu/drm/amd/amdgpu/vega10_ih.c  | 1 +
  3 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
index 7b1762b1c595..965875b8bf93 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
@@ -55,6 +55,7 @@ struct amdgpu_iv_entry {
unsigned timestamp_src;
unsigned pasid;
unsigned pasid_src;
+   unsigned ctxid; /* for SMC to Host interrupt */
unsigned src_data[AMDGPU_IRQ_SRC_DATA_MAX_SIZE_DW];
const uint32_t *iv_entry;
  };
diff --git a/drivers/gpu/drm/amd/amdgpu/navi10_ih.c 
b/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
index f97857ed3c7e..932dc3eabbe2 100644
--- a/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
@@ -470,6 +470,7 @@ static void navi10_ih_decode_iv(struct amdgpu_device *adev,
entry->timestamp_src = dw[2] >> 31;
entry->pasid = dw[3] & 0x;
entry->pasid_src = dw[3] >> 31;
+   entry->ctxid = dw[4];
entry->src_data[0] = dw[4];
entry->src_data[1] = dw[5];
entry->src_data[2] = dw[6];
diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c 
b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
index 407c6093c2ec..e46d1f9f62ca 100644
--- a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
@@ -460,6 +460,7 @@ static void vega10_ih_decode_iv(struct amdgpu_device *adev,
entry->timestamp_src = dw[2] >> 31;
entry->pasid = dw[3] & 0x;
entry->pasid_src = dw[3] >> 31;
+   entry->ctxid = dw[4];
entry->src_data[0] = dw[4];
entry->src_data[1] = dw[5];
entry->src_data[2] = dw[6];


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

[PATCH 2/2] drm/amd/powerplay: report correct AC/DC event based on ctxid

2020-05-08 Thread Evan Quan 
'ctxid' is used to distinguish different events raised from SMC.
0x3 and 0x4 are for AC and DC power mode.

Change-Id: I7dbcb053fe9cea7c70e0a33afc2115b3745f6186
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c 
b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
index ab727f33b8d9..dcd3a54d18a3 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
@@ -1522,6 +1522,7 @@ static int smu_v11_0_irq_process(struct amdgpu_device 
*adev,
 {
uint32_t client_id = entry->client_id;
uint32_t src_id = entry->src_id;
+   uint32_t ctxid = entry->ctxid;
 
if (client_id == SOC15_IH_CLIENTID_THM) {
switch (src_id) {
@@ -1547,8 +1548,18 @@ static int smu_v11_0_irq_process(struct amdgpu_device 
*adev,
 
}
} else if (client_id == SOC15_IH_CLIENTID_MP1) {
-   if (src_id == 0xfe)
-   smu_v11_0_ack_ac_dc_interrupt(>smu);
+   if (src_id == 0xfe) {
+   switch (ctxid) {
+   case 0x3:
+   dev_info(adev->dev, "Switched to AC mode!\n");
+   smu_v11_0_ack_ac_dc_interrupt(>smu);
+   break;
+   case 0x4:
+   dev_info(adev->dev, "Switched to DC mode!\n");
+   smu_v11_0_ack_ac_dc_interrupt(>smu);
+   break;
+   }
+   }
}
 
return 0;
-- 
2.26.2

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

[PATCH 1/2] drm/amdgpu: decode the ctxid for SMC to host IH

2020-05-08 Thread Evan Quan 
Driver needs that to tell the different events raised from SMC.

Change-Id: I0e44e22527182fbb45a2db4fc3b1cd73fb17ba33
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h | 1 +
 drivers/gpu/drm/amd/amdgpu/navi10_ih.c  | 1 +
 drivers/gpu/drm/amd/amdgpu/vega10_ih.c  | 1 +
 3 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
index 7b1762b1c595..965875b8bf93 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
@@ -55,6 +55,7 @@ struct amdgpu_iv_entry {
unsigned timestamp_src;
unsigned pasid;
unsigned pasid_src;
+   unsigned ctxid; /* for SMC to Host interrupt */
unsigned src_data[AMDGPU_IRQ_SRC_DATA_MAX_SIZE_DW];
const uint32_t *iv_entry;
 };
diff --git a/drivers/gpu/drm/amd/amdgpu/navi10_ih.c 
b/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
index f97857ed3c7e..932dc3eabbe2 100644
--- a/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
@@ -470,6 +470,7 @@ static void navi10_ih_decode_iv(struct amdgpu_device *adev,
entry->timestamp_src = dw[2] >> 31;
entry->pasid = dw[3] & 0x;
entry->pasid_src = dw[3] >> 31;
+   entry->ctxid = dw[4];
entry->src_data[0] = dw[4];
entry->src_data[1] = dw[5];
entry->src_data[2] = dw[6];
diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c 
b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
index 407c6093c2ec..e46d1f9f62ca 100644
--- a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
@@ -460,6 +460,7 @@ static void vega10_ih_decode_iv(struct amdgpu_device *adev,
entry->timestamp_src = dw[2] >> 31;
entry->pasid = dw[3] & 0x;
entry->pasid_src = dw[3] >> 31;
+   entry->ctxid = dw[4];
entry->src_data[0] = dw[4];
entry->src_data[1] = dw[5];
entry->src_data[2] = dw[6];
-- 
2.26.2

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

Re: [PATCH 06/36] drm/amdgpu: use the unlocked drm_gem_object_put

2020-05-08 Thread Emil Velikov 
On Fri, 8 May 2020 at 09:16, Christian König  wrote:
>
> Am 07.05.20 um 20:03 schrieb Sam Ravnborg:
> > Hi Emil.
> >
> > On Thu, May 07, 2020 at 04:07:52PM +0100, Emil Velikov wrote:
> >> From: Emil Velikov 
> >>
> >> The driver does not hold struct_mutex, thus using the locked version of
> >> the helper is incorrect.
> >>
> >> Cc: Alex Deucher 
> >> Cc: Christian König 
> >> Cc: amd-gfx@lists.freedesktop.org
> >> Fixes: a39414716ca0 ("drm/amdgpu: add independent DMA-buf import v9"):
> >> Signed-off-by: Emil Velikov 
> >> ---
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c 
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> >> index 43d8ed7dbd00..652c57a3b847 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> >> @@ -587,7 +587,7 @@ struct drm_gem_object *amdgpu_gem_prime_import(struct 
> >> drm_device *dev,
> >>  attach = dma_buf_dynamic_attach(dma_buf, dev->dev,
> >>  _dma_buf_attach_ops, obj);
> >>  if (IS_ERR(attach)) {
> >> -drm_gem_object_put(obj);
> >> +drm_gem_object_put_unlocked(obj);
> >>  return ERR_CAST(attach);
> >>  }
> > Likewise previous patch.
> > Drop this as the patch is correct after the rename a few pathces later.
>
> Well this is a bug fix in the error path and should probably be back
> ported, so having a separate patch is certainly a good idea.
>
Precisely the goal here. The fixes tag should allow Greg and team to
pick/port it where applicable.

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

RE: [PATCH] Series to disallow XGMI link power down during RAS XGMI error injection

2020-05-08 Thread Clements, John 
[AMD Public Use]

Responses inline

From: Chen, Guchun 
Sent: Friday, May 8, 2020 3:11 PM
To: Clements, John ; amd-gfx@lists.freedesktop.org; 
Zhang, Hawking 
Subject: RE: [PATCH] Series to disallow XGMI link power down during RAS XGMI 
error injection


[AMD Public Use]

Patch 1,
1. maybe it's better to split it into 2, one is for header file change, and 
another is functional patch.
2.  Can below codes be merged to one return instead? Variable 'en' can serve 
the function input in smu_send_smc_msg_with_param.
[JC] I think what you are suggesting would probably work, but my concern is 
that it is not immediate clear to me if bool ec will compile to 1. The spec for 
the SMU cmd defines 1 to enable and 0 to disable
if (en)
+   return smu_send_smc_msg_with_param(smu,
+   
SMU_MSG_GmiPwrDnControl,
+   
1,
+   
NULL);
+
+return smu_send_smc_msg_with_param(smu,
+ 
SMU_MSG_GmiPwrDnControl,
+ 0,
+ NULL);

Patch 2: Reviewed-by: Guchun Chen 
guchun.c...@amd.com

Patch 3:
1.function amdgpu_ras_error_inject_xgmi should be one static function? [JC] 
That makes sense, I'll update function to be static
2. Regarding below return case, is there one dpm_allow_xgmi_power_down leak? As 
we have disabled xgmi link power down.
[JC] I intentionally did this, when the RAS int is triggered I don't want to 
create any races conditions to access the SMU to perform functions and entering 
BACO reset, the XGMI per link settings will be restored anyway after the BACO 
reset.
[JC] That being said I understand that this looks like it could be a bug, I'll 
add comments there to clarify the sequence
+if (amdgpu_ras_intr_triggered())
+   return ret;

Regards,
Guchun

From: amd-gfx 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 On Behalf Of Clements, John
Sent: Friday, May 8, 2020 2:51 PM
To: amd-gfx@lists.freedesktop.org; Zhang, 
Hawking mailto:hawking.zh...@amd.com>>
Subject: [PATCH] Series to disallow XGMI link power down during RAS XGMI error 
injection


[AMD Official Use Only - Internal Distribution Only]

Submitting 3 patches for review:

  *   Added host to SMU FW cmd to enable/disable XGMI link power down
  *   Added DPM function for XGMI link power down control
  *   Disable XGMI link power down prior to issuing a XGMI RAS error
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: implement soft_recovery for gfx10

2020-05-08 Thread Christian König 

Am 05.05.20 um 18:42 schrieb Alex Deucher:

Same as gfx9.  This allows us to kill the waves for hung
shaders.

Signed-off-by: Alex Deucher 


Reviewed-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 14 ++
  1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index ddb485e1e963..27c63a8f698c 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -7690,6 +7690,19 @@ static void 
gfx_v10_0_ring_emit_reg_write_reg_wait(struct amdgpu_ring *ring,
   ref, mask);
  }
  
+static void gfx_v10_0_ring_soft_recovery(struct amdgpu_ring *ring,

+unsigned vmid)
+{
+   struct amdgpu_device *adev = ring->adev;
+   uint32_t value = 0;
+
+   value = REG_SET_FIELD(value, SQ_CMD, CMD, 0x03);
+   value = REG_SET_FIELD(value, SQ_CMD, MODE, 0x01);
+   value = REG_SET_FIELD(value, SQ_CMD, CHECK_VMID, 1);
+   value = REG_SET_FIELD(value, SQ_CMD, VM_ID, vmid);
+   WREG32_SOC15(GC, 0, mmSQ_CMD, value);
+}
+
  static void
  gfx_v10_0_set_gfx_eop_interrupt_state(struct amdgpu_device *adev,
  uint32_t me, uint32_t pipe,
@@ -8105,6 +8118,7 @@ static const struct amdgpu_ring_funcs 
gfx_v10_0_ring_funcs_gfx = {
.emit_wreg = gfx_v10_0_ring_emit_wreg,
.emit_reg_wait = gfx_v10_0_ring_emit_reg_wait,
.emit_reg_write_reg_wait = gfx_v10_0_ring_emit_reg_write_reg_wait,
+   .soft_recovery = gfx_v10_0_ring_soft_recovery,
.emit_mem_sync = gfx_v10_0_emit_mem_sync,
  };
  


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

Re: [PATCH 1/1] drm/amdgpu: cleanup sysfs file handling

2020-05-08 Thread Christian König 

Am 08.05.20 um 10:48 schrieb Nirmoy Das:

Create sysfs file using attributes.

Signed-off-by: Nirmoy Das 
Reviewed-by: Alex Deucher 


Reviewed-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 36 --
  1 file changed, 12 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index bf302c799832..cc41e8f5ad14 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2918,6 +2918,14 @@ static int amdgpu_device_get_job_timeout_settings(struct 
amdgpu_device *adev)
return ret;
  }
  
+static const struct attribute *amdgpu_dev_attributes[] = {

+   _attr_product_name.attr,
+   _attr_product_number.attr,
+   _attr_serial_number.attr,
+   _attr_pcie_replay_count.attr,
+   NULL
+};
+
  /**
   * amdgpu_device_init - initialize the driver
   *
@@ -3267,27 +3275,9 @@ int amdgpu_device_init(struct amdgpu_device *adev,
queue_delayed_work(system_wq, >delayed_init_work,
   msecs_to_jiffies(AMDGPU_RESUME_MS));
  
-	r = device_create_file(adev->dev, _attr_pcie_replay_count);

-   if (r) {
-   dev_err(adev->dev, "Could not create pcie_replay_count");
-   return r;
-   }
-
-   r = device_create_file(adev->dev, _attr_product_name);
+   r = sysfs_create_files(>dev->kobj, amdgpu_dev_attributes);
if (r) {
-   dev_err(adev->dev, "Could not create product_name");
-   return r;
-   }
-
-   r = device_create_file(adev->dev, _attr_product_number);
-   if (r) {
-   dev_err(adev->dev, "Could not create product_number");
-   return r;
-   }
-
-   r = device_create_file(adev->dev, _attr_serial_number);
-   if (r) {
-   dev_err(adev->dev, "Could not create serial_number");
+   dev_err(adev->dev, "Could not create amdgpu device attr\n");
return r;
}
  
@@ -3370,12 +3360,10 @@ void amdgpu_device_fini(struct amdgpu_device *adev)

adev->rmmio = NULL;
amdgpu_device_doorbell_fini(adev);
  
-	device_remove_file(adev->dev, _attr_pcie_replay_count);

if (adev->ucode_sysfs_en)
amdgpu_ucode_sysfs_fini(adev);
-   device_remove_file(adev->dev, _attr_product_name);
-   device_remove_file(adev->dev, _attr_product_number);
-   device_remove_file(adev->dev, _attr_serial_number);
+
+   sysfs_remove_files(>dev->kobj, amdgpu_dev_attributes);
if (IS_ENABLED(CONFIG_PERF_EVENTS))
amdgpu_pmu_fini(adev);
if (amdgpu_discovery && adev->asic_type >= CHIP_NAVI10)


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

[PATCH 1/1] drm/amdgpu: cleanup sysfs file handling

2020-05-08 Thread Nirmoy Das 
Create sysfs file using attributes.

Signed-off-by: Nirmoy Das 
Reviewed-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 36 --
 1 file changed, 12 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index bf302c799832..cc41e8f5ad14 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2918,6 +2918,14 @@ static int amdgpu_device_get_job_timeout_settings(struct 
amdgpu_device *adev)
return ret;
 }
 
+static const struct attribute *amdgpu_dev_attributes[] = {
+   _attr_product_name.attr,
+   _attr_product_number.attr,
+   _attr_serial_number.attr,
+   _attr_pcie_replay_count.attr,
+   NULL
+};
+
 /**
  * amdgpu_device_init - initialize the driver
  *
@@ -3267,27 +3275,9 @@ int amdgpu_device_init(struct amdgpu_device *adev,
queue_delayed_work(system_wq, >delayed_init_work,
   msecs_to_jiffies(AMDGPU_RESUME_MS));
 
-   r = device_create_file(adev->dev, _attr_pcie_replay_count);
-   if (r) {
-   dev_err(adev->dev, "Could not create pcie_replay_count");
-   return r;
-   }
-
-   r = device_create_file(adev->dev, _attr_product_name);
+   r = sysfs_create_files(>dev->kobj, amdgpu_dev_attributes);
if (r) {
-   dev_err(adev->dev, "Could not create product_name");
-   return r;
-   }
-
-   r = device_create_file(adev->dev, _attr_product_number);
-   if (r) {
-   dev_err(adev->dev, "Could not create product_number");
-   return r;
-   }
-
-   r = device_create_file(adev->dev, _attr_serial_number);
-   if (r) {
-   dev_err(adev->dev, "Could not create serial_number");
+   dev_err(adev->dev, "Could not create amdgpu device attr\n");
return r;
}
 
@@ -3370,12 +3360,10 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
adev->rmmio = NULL;
amdgpu_device_doorbell_fini(adev);
 
-   device_remove_file(adev->dev, _attr_pcie_replay_count);
if (adev->ucode_sysfs_en)
amdgpu_ucode_sysfs_fini(adev);
-   device_remove_file(adev->dev, _attr_product_name);
-   device_remove_file(adev->dev, _attr_product_number);
-   device_remove_file(adev->dev, _attr_serial_number);
+
+   sysfs_remove_files(>dev->kobj, amdgpu_dev_attributes);
if (IS_ENABLED(CONFIG_PERF_EVENTS))
amdgpu_pmu_fini(adev);
if (amdgpu_discovery && adev->asic_type >= CHIP_NAVI10)
-- 
2.26.2

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

Re: [PATCH 06/36] drm/amdgpu: use the unlocked drm_gem_object_put

2020-05-08 Thread Christian König 

Am 07.05.20 um 20:03 schrieb Sam Ravnborg:

Hi Emil.

On Thu, May 07, 2020 at 04:07:52PM +0100, Emil Velikov wrote:

From: Emil Velikov 

The driver does not hold struct_mutex, thus using the locked version of
the helper is incorrect.

Cc: Alex Deucher 
Cc: Christian König 
Cc: amd-gfx@lists.freedesktop.org
Fixes: a39414716ca0 ("drm/amdgpu: add independent DMA-buf import v9"):
Signed-off-by: Emil Velikov 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 43d8ed7dbd00..652c57a3b847 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -587,7 +587,7 @@ struct drm_gem_object *amdgpu_gem_prime_import(struct 
drm_device *dev,
attach = dma_buf_dynamic_attach(dma_buf, dev->dev,
_dma_buf_attach_ops, obj);
if (IS_ERR(attach)) {
-   drm_gem_object_put(obj);
+   drm_gem_object_put_unlocked(obj);
return ERR_CAST(attach);
}

Likewise previous patch.
Drop this as the patch is correct after the rename a few pathces later.


Well this is a bug fix in the error path and should probably be back 
ported, so having a separate patch is certainly a good idea.


Christian.



Sam

  
--

2.25.1

___
dri-devel mailing list
dri-de...@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-develdata=02%7C01%7Cchristian.koenig%40amd.com%7C41036bd7ab804d8bebdf08d7f2b0e938%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637244713989571165sdata=d0LysMNmsc%2BSzSkYsZDmE43azU0nmAb%2B1tFHn%2BMNxDg%3Dreserved=0


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

Re: [PATCH 06/36] drm/amdgpu: use the unlocked drm_gem_object_put

2020-05-08 Thread Christian König 

Am 07.05.20 um 17:07 schrieb Emil Velikov:

From: Emil Velikov 

The driver does not hold struct_mutex, thus using the locked version of
the helper is incorrect.

Cc: Alex Deucher 
Cc: Christian König 
Cc: amd-gfx@lists.freedesktop.org
Fixes: a39414716ca0 ("drm/amdgpu: add independent DMA-buf import v9"):
Signed-off-by: Emil Velikov 


Reviewed-by: Christian König 


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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 43d8ed7dbd00..652c57a3b847 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -587,7 +587,7 @@ struct drm_gem_object *amdgpu_gem_prime_import(struct 
drm_device *dev,
attach = dma_buf_dynamic_attach(dma_buf, dev->dev,
_dma_buf_attach_ops, obj);
if (IS_ERR(attach)) {
-   drm_gem_object_put(obj);
+   drm_gem_object_put_unlocked(obj);
return ERR_CAST(attach);
}
  


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

RE: [PATCH] Series to disallow XGMI link power down during RAS XGMI error injection

2020-05-08 Thread Chen, Guchun 
[AMD Public Use]

Patch 1,
1. maybe it's better to split it into 2, one is for header file change, and 
another is functional patch.
2.  Can below codes be merged to one return instead? Variable 'en' can serve 
the function input in smu_send_smc_msg_with_param.

if (en)
+   return smu_send_smc_msg_with_param(smu,
+   
SMU_MSG_GmiPwrDnControl,
+   
1,
+   
NULL);
+
+return smu_send_smc_msg_with_param(smu,
+ 
SMU_MSG_GmiPwrDnControl,
+ 0,
+ NULL);

Patch 2: Reviewed-by: Guchun Chen 
guchun.c...@amd.com

Patch 3:
1.function amdgpu_ras_error_inject_xgmi should be one static function?
2. Regarding below return case, is there one dpm_allow_xgmi_power_down leak? As 
we have disabled xgmi link power down.

+if (amdgpu_ras_intr_triggered())
+   return ret;

Regards,
Guchun

From: amd-gfx  On Behalf Of Clements, 
John
Sent: Friday, May 8, 2020 2:51 PM
To: amd-gfx@lists.freedesktop.org; Zhang, Hawking 
Subject: [PATCH] Series to disallow XGMI link power down during RAS XGMI error 
injection


[AMD Official Use Only - Internal Distribution Only]

Submitting 3 patches for review:

  *   Added host to SMU FW cmd to enable/disable XGMI link power down
  *   Added DPM function for XGMI link power down control
  *   Disable XGMI link power down prior to issuing a XGMI RAS error
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] Series to disallow XGMI link power down during RAS XGMI error injection

2020-05-08 Thread Zhang, Hawking 
[AMD Official Use Only - Internal Distribution Only]

Patch #1
+ pr_err("[AllowXgmiPowerDown] failed!\n");
Please replace pr_xxx with dev_xxx to explicitly show device bdf in kernel 
message especially for mGPU use case

+ if (smu_version < 0x365000) {
Let's check pmfw 54.23.0 for backward compatibility.

Other than that, the series is

Reviewed-by: Hawking Zhang 

Regards,
Hawking
From: Clements, John 
Sent: Friday, May 8, 2020 14:51
To: amd-gfx@lists.freedesktop.org; Zhang, Hawking 
Subject: [PATCH] Series to disallow XGMI link power down during RAS XGMI error 
injection


[AMD Official Use Only - Internal Distribution Only]

Submitting 3 patches for review:

  *   Added host to SMU FW cmd to enable/disable XGMI link power down
  *   Added DPM function for XGMI link power down control
  *   Disable XGMI link power down prior to issuing a XGMI RAS error
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx