RE: [bug report] drm/amd/powerplay: add point check to avoid NULL point hang.

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

Thanks. I just sent out a patch to address this.

BR
Evan
-Original Message-
From: amd-gfx  On Behalf Of Dan Carpenter
Sent: Tuesday, August 25, 2020 6:24 PM
To: rex@amd.com
Cc: amd-gfx@lists.freedesktop.org
Subject: [bug report] drm/amd/powerplay: add point check to avoid NULL point 
hang.

Hello Rex Zhu,

The patch 88b8dcbe21fd: "drm/amd/powerplay: add point check to avoid NULL point 
hang." from Dec 11, 2015, leads to the following static checker warning:

drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/hardwaremanager.c:274 
phm_check_smc_update_required_for_display_configuration()
warn: signedness bug returning '(-22)'

drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/hardwaremanager.c
   272  bool phm_check_smc_update_required_for_display_configuration(struct 
pp_hwmgr *hwmgr)
   273  {
   274  PHM_FUNC_CHECK(hwmgr);

PHM_FUNC_CHECK() has a hiddren -EINVAL return that becomes true when casted to 
bool.

   275  if (hwmgr->pp_one_vf)
   276  return false;
   277
   278  if 
(hwmgr->hwmgr_func->check_smc_update_required_for_display_configuration == NULL)
   279  return false;
   280
   281  return 
hwmgr->hwmgr_func->check_smc_update_required_for_display_configuration(hwmgr);
   282  }

regards,
dan carpenter
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cevan.quan%40amd.com%7Cd3cadb4b92e04c5d4eae08d848e111a0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637339478834526298&sdata=236vygLQdbMVXk6XBDaayrTw2qODhcmTCFvtl0%2B95IM%3D&reserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH] drm/amd/pm: suppress static checker warning

2020-08-25 Thread Evan Quan
Suppress the warning below:
drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/hardwaremanager.c:274 
phm_check_smc_update_required_for_display_configuration()
warn: signedness bug returning '(-22)'

Change-Id: If50e39fe401c16d981d917ef7d8d5ea81d6538df
Reported-by: Dan Carpenter 
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/pm/powerplay/hwmgr/hardwaremanager.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/hardwaremanager.c 
b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/hardwaremanager.c
index 9454ab50f9a1..1f9b9facdf1f 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/hardwaremanager.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/hardwaremanager.c
@@ -271,7 +271,10 @@ int phm_start_thermal_controller(struct pp_hwmgr *hwmgr)
 
 bool phm_check_smc_update_required_for_display_configuration(struct pp_hwmgr 
*hwmgr)
 {
-   PHM_FUNC_CHECK(hwmgr);
+   if (hwmgr == NULL ||
+   hwmgr->hwmgr_func == NULL)
+   return false;
+
if (hwmgr->pp_one_vf)
return false;
 
-- 
2.28.0

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


Re: [PATCH] drm/amdgpu: report DC not supported if virtual display is enabled (v2)

2020-08-25 Thread Luben Tuikov
On 2020-08-25 10:34 a.m., Nirmoy wrote:
> 
> On 8/25/20 4:18 PM, Alex Deucher wrote:
>> virtual display is non-atomic so report false to avoid checking
> 
> With below nitpick fixed, Acked-by: Nirmoy Das 

Another nitpick: "below" is not an adjective. It's a preposition or in
this case as an adverb:

"With the nitpick below fixed,"...

Regards,
Luben

> 
> virtual --> Virtual
> 
> 
> Nirmoy
> 
> 
>> atomic state and other atomic things at runtime.
>>
>> v2: squash into the sr-iov check
>>
>> Signed-off-by: Alex Deucher 
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 5a948edcc93c..8f37f9f99105 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -2861,7 +2861,7 @@ bool amdgpu_device_asic_has_dc_support(enum 
>> amd_asic_type asic_type)
>>*/
>>   bool amdgpu_device_has_dc_support(struct amdgpu_device *adev)
>>   {
>> -if (amdgpu_sriov_vf(adev))
>> +if (amdgpu_sriov_vf(adev) || adev->enable_virtual_display)
>>  return false;
>>   
>>  return amdgpu_device_asic_has_dc_support(adev->asic_type);
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cluben.tuikov%40amd.com%7Ca80cd8a3f9b34b19d07708d849036b25%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637339626337747219&sdata=6xwRnYCESQNachEMmT2jpVjKTsDg4RkwQKpMsOTsSkA%3D&reserved=0
> 

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


[PATCH] drm/amdkfd: Add GPU reset SMI event

2020-08-25 Thread Mukul Joshi
Add support for reporting GPU reset events through SMI. KFD
would report both pre and post GPU reset events.

Signed-off-by: Mukul Joshi 
---
 drivers/gpu/drm/amd/amdkfd/kfd_device.c |  4 +++
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h   |  2 ++
 drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c | 30 +
 drivers/gpu/drm/amd/amdkfd/kfd_smi_events.h |  1 +
 include/uapi/linux/kfd_ioctl.h  |  2 ++
 5 files changed, 39 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index e1cd6599529f..aad1ecfa1239 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -812,6 +812,8 @@ int kgd2kfd_pre_reset(struct kfd_dev *kfd)
if (!kfd->init_complete)
return 0;
 
+   kfd_smi_event_update_gpu_reset(kfd, false);
+
kfd->dqm->ops.pre_reset(kfd->dqm);
 
kgd2kfd_suspend(kfd, false);
@@ -833,6 +835,8 @@ int kgd2kfd_post_reset(struct kfd_dev *kfd)
if (!kfd->init_complete)
return 0;
 
+   kfd_smi_event_update_gpu_reset(kfd, true);
+
ret = kfd_resume(kfd);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 18bc711f97ae..b1a2979e086f 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -312,6 +312,8 @@ struct kfd_dev {
/* Clients watching SMI events */
struct list_head smi_clients;
spinlock_t smi_lock;
+
+   uint64_t reset_seq_num;
 };
 
 enum kfd_mempool {
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
index 4d4b6e3ab697..448abfdde230 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
@@ -174,6 +174,36 @@ static void add_event_to_kfifo(struct kfd_dev *dev, 
unsigned int smi_event,
rcu_read_unlock();
 }
 
+void kfd_smi_event_update_gpu_reset(struct kfd_dev *dev, bool post_reset)
+{
+   /*
+* GpuReset msg = Reset seq number (incremented for
+* every reset message sent before GPU reset).
+* 1 byte event + 1 byte space + 16 bytes seq num +
+* 1 byte \n + 1 byte \0 = 20
+*/
+   char fifo_in[20];
+   int len;
+   unsigned int event;
+
+   if (list_empty(&dev->smi_clients)) {
+   return;
+   }
+
+   memset(fifo_in, 0x0, sizeof(fifo_in));
+
+   if (post_reset) {
+   event = KFD_SMI_EVENT_GPU_POST_RESET;
+   } else {
+   event = KFD_SMI_EVENT_GPU_PRE_RESET;
+   ++(dev->reset_seq_num);
+   }
+
+   len = snprintf(fifo_in, 4, "%x %llx\n", event, dev->reset_seq_num);
+
+   add_event_to_kfifo(dev, event, fifo_in, len);
+}
+
 void kfd_smi_event_update_thermal_throttling(struct kfd_dev *dev,
 uint32_t throttle_bitmask)
 {
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.h
index 15537b2cccb5..b9b0438202e2 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.h
@@ -27,5 +27,6 @@ int kfd_smi_event_open(struct kfd_dev *dev, uint32_t *fd);
 void kfd_smi_event_update_vmfault(struct kfd_dev *dev, uint16_t pasid);
 void kfd_smi_event_update_thermal_throttling(struct kfd_dev *dev,
 uint32_t throttle_bitmask);
+void kfd_smi_event_update_gpu_reset(struct kfd_dev *dev, bool post_reset);
 
 #endif
diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
index cb1f963a84e0..8b7368bfbd84 100644
--- a/include/uapi/linux/kfd_ioctl.h
+++ b/include/uapi/linux/kfd_ioctl.h
@@ -453,6 +453,8 @@ enum kfd_smi_event {
 KFD_SMI_EVENT_NONE = 0, /* not used */
 KFD_SMI_EVENT_VMFAULT = 1, /* event start counting at 1 */
 KFD_SMI_EVENT_THERMAL_THROTTLE = 2,
+   KFD_SMI_EVENT_GPU_PRE_RESET = 3,
+   KFD_SMI_EVENT_GPU_POST_RESET = 4,
 };
 
 #define KFD_SMI_EVENT_MASK_FROM_INDEX(i) (1ULL << ((i) - 1))
-- 
2.17.1

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


[PATCH] drm/radeon: Prefer lower feedback dividers

2020-08-25 Thread Kai-Heng Feng
Commit 2e26ccb119bd ("drm/radeon: prefer lower reference dividers")
fixed screen flicker for HP Compaq nx9420 but breaks other laptops like
Asus X50SL.

Turns out we also need to favor lower feedback dividers.

Users confirmed this change fixes the regression and doesn't regress the
original fix.

Fixes: 2e26ccb119bd ("drm/radeon: prefer lower reference dividers")
BugLink: https://bugs.launchpad.net/bugs/1791312
BugLink: https://bugs.launchpad.net/bugs/1861554
Signed-off-by: Kai-Heng Feng 
---
 drivers/gpu/drm/radeon/radeon_display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_display.c 
b/drivers/gpu/drm/radeon/radeon_display.c
index e0ae911ef427..7b69d6dfe44a 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -933,7 +933,7 @@ static void avivo_get_fb_ref_div(unsigned nom, unsigned 
den, unsigned post_div,
 
/* get matching reference and feedback divider */
*ref_div = min(max(den/post_div, 1u), ref_div_max);
-   *fb_div = DIV_ROUND_CLOSEST(nom * *ref_div * post_div, den);
+   *fb_div = max(nom * *ref_div * post_div / den, 1u);
 
/* limit fb divider to its maximum */
if (*fb_div > fb_div_max) {
-- 
2.17.1

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


Re: [PATCH 1/3] drm/amdgpu/si: Fix buffer overflow in si_get_register_value()

2020-08-25 Thread Dan Carpenter
On Tue, Aug 25, 2020 at 11:53:25AM -0400, Alex Deucher wrote:
> On Tue, Aug 25, 2020 at 7:21 AM Dan Carpenter  
> wrote:
> >
> > The values for "se_num" and "sh_num" come from the user in the ioctl.
> > They can be in the 0-255 range but if they're more than
> > AMDGPU_GFX_MAX_SE (4) or AMDGPU_GFX_MAX_SH_PER_SE (2) then it results in
> > an out of bounds read.
> >
> > I split this function into to two to make the error handling simpler.
> >
> > Fixes: dd5dfa61b4ff ("drm/amdgpu: refine si_read_register")
> > Signed-off-by: Dan Carpenter 
> 
> Good catch.  This is more defensive, but It's a much simpler check to
> validate these in the caller.  See the attached patch.
> 

That works too.

Acked-by: Dan Carpenter 

regards,
dan carpenter

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


Re: [PATCH] drm/amdgpu/dc: Require primary plane to be enabled whenever the CRTC is

2020-08-25 Thread Kazlauskas, Nicholas

On 2020-08-22 5:59 a.m., Michel Dänzer wrote:

On 2020-08-21 8:07 p.m., Kazlauskas, Nicholas wrote:

On 2020-08-21 12:57 p.m., Michel Dänzer wrote:

From: Michel Dänzer 

Don't check drm_crtc_state::active for this either, per its
documentation in include/drm/drm_crtc.h:

   * Hence drivers must not consult @active in their various
   * &drm_mode_config_funcs.atomic_check callback to reject an atomic
   * commit.

The atomic helpers disable the CRTC as needed for disabling the primary
plane.

This prevents at least the following problems if the primary plane gets
disabled (e.g. due to destroying the FB assigned to the primary plane,
as happens e.g. with mutter in Wayland mode):

* Toggling CRTC active to 1 failed if the cursor plane was enabled
    (e.g. via legacy DPMS property & cursor ioctl).
* Enabling the cursor plane failed, e.g. via the legacy cursor ioctl.


We previously had the requirement that the primary plane must be enabled
but some userspace expects that they can enable just the overlay plane
without anything else.

I think the chromuiumos atomictest validates that this works as well:

So is DRM going forward then with the expectation that this is wrong
behavior from userspace?

We require at least one plane to be enabled to display a cursor, but it
doesn't necessarily need to be the primary.


It's a "pick your poison" situation:

1) Currently the checks are invalid (atomic_check must not decide based
on drm_crtc_state::active), and it's easy for legacy KMS userspace to
accidentally hit errors trying to enable/move the cursor or switch DPMS
off → on.

2) Accurately rejecting only atomic states where the cursor plane is
enabled but all other planes are off would break the KMS helper code,
which can only deal with the "CRTC on & primary plane off is not
allowed" case specifically.

3) This patch addresses 1) & 2) but may break existing atomic userspace
which wants to enable an overlay plane while disabling the primary plane.


I do think in principle atomic userspace is expected to handle case 3)
and leave the primary plane enabled. However, this is not ideal from an
energy consumption PoV. Therefore, here's another idea for a possible
way out of this quagmire:

amdgpu_dm does not reject any atomic states based on which planes are
enabled in it. If the cursor plane is enabled but all other planes are
off, amdgpu_dm internally either:

a) Enables an overlay plane and makes it invisible, e.g. by assigning a
minimum size FB with alpha = 0.

b) Enables the primary plane and assigns a minimum size FB (scaled up to
the required size) containing all black, possibly using compression.
(Trying to minimize the memory bandwidth)


Does either of these seem feasible? If both do, which one would be
preferable?




It's really the same solution since DCN doesn't make a distinction 
between primary or overlay planes in hardware. DCE doesn't have overlay 
planes enabled so this is not relevant there.


The old behavior (pre 5.1?) was to silently accept the commit even 
though the screen would be completely black instead of outright 
rejecting the commit.


I almost wonder if that makes more sense in the short term here since 
the only "userspace" affected here is IGT. We'll fail the CRC checks, 
but no userspace actually tries to actively use a cursor with no primary 
plane enabled from my understanding.


In the long term I think we can work on getting cursor actually on the 
screen in this case, though I can't say I really like having to reserve 
some small buffer (eg. 16x16) for allowing lightup on this corner case.


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


Re: [PATCH 1/3] drm/amdgpu/si: Fix buffer overflow in si_get_register_value()

2020-08-25 Thread Alex Deucher
On Tue, Aug 25, 2020 at 7:21 AM Dan Carpenter  wrote:
>
> The values for "se_num" and "sh_num" come from the user in the ioctl.
> They can be in the 0-255 range but if they're more than
> AMDGPU_GFX_MAX_SE (4) or AMDGPU_GFX_MAX_SH_PER_SE (2) then it results in
> an out of bounds read.
>
> I split this function into to two to make the error handling simpler.
>
> Fixes: dd5dfa61b4ff ("drm/amdgpu: refine si_read_register")
> Signed-off-by: Dan Carpenter 

Good catch.  This is more defensive, but It's a much simpler check to
validate these in the caller.  See the attached patch.

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/si.c | 157 +---
>  1 file changed, 85 insertions(+), 72 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/si.c b/drivers/gpu/drm/amd/amdgpu/si.c
> index e330884edd19..ccf39a6932ae 100644
> --- a/drivers/gpu/drm/amd/amdgpu/si.c
> +++ b/drivers/gpu/drm/amd/amdgpu/si.c
> @@ -1051,81 +1051,90 @@ static struct amdgpu_allowed_register_entry 
> si_allowed_read_registers[] = {
> {PA_SC_RASTER_CONFIG, true},
>  };
>
> -static uint32_t si_get_register_value(struct amdgpu_device *adev,
> - bool indexed, u32 se_num,
> - u32 sh_num, u32 reg_offset)
> -{
> -   if (indexed) {
> -   uint32_t val;
> -   unsigned se_idx = (se_num == 0x) ? 0 : se_num;
> -   unsigned sh_idx = (sh_num == 0x) ? 0 : sh_num;
> -
> -   switch (reg_offset) {
> -   case mmCC_RB_BACKEND_DISABLE:
> -   return 
> adev->gfx.config.rb_config[se_idx][sh_idx].rb_backend_disable;
> -   case mmGC_USER_RB_BACKEND_DISABLE:
> -   return 
> adev->gfx.config.rb_config[se_idx][sh_idx].user_rb_backend_disable;
> -   case mmPA_SC_RASTER_CONFIG:
> -   return 
> adev->gfx.config.rb_config[se_idx][sh_idx].raster_config;
> -   }
> +static int si_get_register_value_indexed(struct amdgpu_device *adev,
> + u32 se_num, u32 sh_num,
> + u32 reg_offset, u32 *value)
> +{
> +   unsigned se_idx = (se_num == 0x) ? 0 : se_num;
> +   unsigned sh_idx = (sh_num == 0x) ? 0 : sh_num;
>
> -   mutex_lock(&adev->grbm_idx_mutex);
> -   if (se_num != 0x || sh_num != 0x)
> -   amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 
> 0x);
> +   if (se_idx >= AMDGPU_GFX_MAX_SE ||
> +   sh_idx >= AMDGPU_GFX_MAX_SH_PER_SE)
> +   return -EINVAL;
>
> -   val = RREG32(reg_offset);
> +   switch (reg_offset) {
> +   case mmCC_RB_BACKEND_DISABLE:
> +   *value = 
> adev->gfx.config.rb_config[se_idx][sh_idx].rb_backend_disable;
> +   return 0;
> +   case mmGC_USER_RB_BACKEND_DISABLE:
> +   *value = 
> adev->gfx.config.rb_config[se_idx][sh_idx].user_rb_backend_disable;
> +   return 0;
> +   case mmPA_SC_RASTER_CONFIG:
> +   *value = 
> adev->gfx.config.rb_config[se_idx][sh_idx].raster_config;
> +   return 0;
> +   }
>
> -   if (se_num != 0x || sh_num != 0x)
> -   amdgpu_gfx_select_se_sh(adev, 0x, 0x, 
> 0x);
> -   mutex_unlock(&adev->grbm_idx_mutex);
> -   return val;
> -   } else {
> -   unsigned idx;
> -
> -   switch (reg_offset) {
> -   case mmGB_ADDR_CONFIG:
> -   return adev->gfx.config.gb_addr_config;
> -   case mmMC_ARB_RAMCFG:
> -   return adev->gfx.config.mc_arb_ramcfg;
> -   case mmGB_TILE_MODE0:
> -   case mmGB_TILE_MODE1:
> -   case mmGB_TILE_MODE2:
> -   case mmGB_TILE_MODE3:
> -   case mmGB_TILE_MODE4:
> -   case mmGB_TILE_MODE5:
> -   case mmGB_TILE_MODE6:
> -   case mmGB_TILE_MODE7:
> -   case mmGB_TILE_MODE8:
> -   case mmGB_TILE_MODE9:
> -   case mmGB_TILE_MODE10:
> -   case mmGB_TILE_MODE11:
> -   case mmGB_TILE_MODE12:
> -   case mmGB_TILE_MODE13:
> -   case mmGB_TILE_MODE14:
> -   case mmGB_TILE_MODE15:
> -   case mmGB_TILE_MODE16:
> -   case mmGB_TILE_MODE17:
> -   case mmGB_TILE_MODE18:
> -   case mmGB_TILE_MODE19:
> -   case mmGB_TILE_MODE20:
> -   case mmGB_TILE_MODE21:
> -   case mmGB_TILE_MODE22:
> -   case mmGB_TILE_MODE23:
> -   case mmGB_TILE_MODE24:
> -   case mmGB_TILE_MODE25:
> -   case mmGB_TILE_MODE26:
> -   case mmGB_TILE_MODE27:
> -   case mmGB_TILE_MODE28:
>

Re: [PATCH] drm/amdgpu/dc: Require primary plane to be enabled whenever the CRTC is

2020-08-25 Thread Michel Dänzer
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 2020-08-24 9:43 a.m., Pekka Paalanen wrote:
> On Sat, 22 Aug 2020 11:59:26 +0200 Michel Dänzer
>  wrote:
>> On 2020-08-21 8:07 p.m., Kazlauskas, Nicholas wrote:
>>> On 2020-08-21 12:57 p.m., Michel Dänzer wrote:
 From: Michel Dänzer 

 Don't check drm_crtc_state::active for this either, per its
 documentation in include/drm/drm_crtc.h:

 * Hence drivers must not consult @active in their various *
 &drm_mode_config_funcs.atomic_check callback to reject an
 atomic * commit.

 The atomic helpers disable the CRTC as needed for disabling
 the primary plane.

 This prevents at least the following problems if the primary
 plane gets disabled (e.g. due to destroying the FB assigned
 to the primary plane, as happens e.g. with mutter in Wayland
 mode):

 * Toggling CRTC active to 1 failed if the cursor plane was
 enabled (e.g. via legacy DPMS property & cursor ioctl). *
 Enabling the cursor plane failed, e.g. via the legacy cursor
 ioctl.
>>>
>>> We previously had the requirement that the primary plane must
>>> be enabled but some userspace expects that they can enable
>>> just the overlay plane without anything else.
>>>
>>> I think the chromuiumos atomictest validates that this works
>>> as well:
>>>
>>> So is DRM going forward then with the expectation that this is
>>> wrong behavior from userspace?
>>>
>>> We require at least one plane to be enabled to display a
>>> cursor, but it doesn't necessarily need to be the primary.
>>
>> It's a "pick your poison" situation:
>>
>> 1) Currently the checks are invalid (atomic_check must not
>> decide based on drm_crtc_state::active), and it's easy for legacy
>> KMS userspace to accidentally hit errors trying to enable/move
>> the cursor or switch DPMS off → on.
>>
>> 2) Accurately rejecting only atomic states where the cursor
>> plane is enabled but all other planes are off would break the
>> KMS helper code, which can only deal with the "CRTC on & primary
>> plane off is not allowed" case specifically.
>>
>> 3) This patch addresses 1) & 2) but may break existing atomic
>> userspace which wants to enable an overlay plane while disabling
>> the primary plane.
>>
>>
>> I do think in principle atomic userspace is expected to handle
>> case 3) and leave the primary plane enabled.
>
> Hi,
>
> my opinion as a userspace developer is that enabling a CRTC
> without a primary plane has traditionally not worked, so userspace
> cannot *rely* on it ever working. I think legacy KMS API does not
> even allow to express that really, or has built-in assumptions that
> it doesn't work - you can call the legacy cursor ioctls, they
> don't fail, but also the CRTC remains off. Correct me if this is
> not true.
>
> Atomic userspace OTOH is required to test the strange (and all!)
> cases like this to see if they work or not, and carry on with a
> fallback if they don't. Userspace that wants to use a CRTC without
> a primary plane likely needs other CRTC properties as well, like
> background color.
>
> I would guess that when two regressions conflict, the older
> userspace would win. Hence legacy KMS using Mutter should keep
> working, while atomic userspace is left with increased power
> consumption. Not using a hardware cursor (Mutter) is much more
> easily an end-user visible regression than slightly shorter
> battery life.
>
> Atomic test commits are allowed to fail at any time for any reason
> and something that previously worked can fail on the next frame or
> on the next kernel, as far as I understand.

All of this basically matches my understanding.


> Sounds like the helpers you refer to are inadequate for your case.
> Can't you fix the helpers in the long run and land this patch as an
> immediate fix?

I'd rather leave working on those helpers to KMS developers. :)


- -- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
-BEGIN PGP SIGNATURE-

iF0EARECAB0WIQSwn681vpFFIZgJURRaga+OatuyAAUCX0UmXAAKCRBaga+Oatuy
AIeNAJoC9UgOrF+qBq08uOyjaV7Vfp+PgACfSp3nXB3un3LUZQxrvaxMAON+eIs=
=Pbd2
-END PGP SIGNATURE-
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH] drm/amdgpu: report DC not supported if virtual display is enabled (v2)

2020-08-25 Thread Nirmoy



On 8/25/20 4:18 PM, Alex Deucher wrote:

virtual display is non-atomic so report false to avoid checking


With below nitpick fixed, Acked-by: Nirmoy Das 

virtual --> Virtual


Nirmoy



atomic state and other atomic things at runtime.

v2: squash into the sr-iov check

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 5a948edcc93c..8f37f9f99105 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2861,7 +2861,7 @@ bool amdgpu_device_asic_has_dc_support(enum amd_asic_type 
asic_type)
   */
  bool amdgpu_device_has_dc_support(struct amdgpu_device *adev)
  {
-   if (amdgpu_sriov_vf(adev))
+   if (amdgpu_sriov_vf(adev) || adev->enable_virtual_display)
return false;
  
  	return amdgpu_device_asic_has_dc_support(adev->asic_type);

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


RE: [PATCH] drm/amdgpu: report DC not supported if virtual display is enabled (v2)

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

Acked-by: Guchun Chen 

Regards,
Guchun

-Original Message-
From: amd-gfx  On Behalf Of Alex Deucher
Sent: Tuesday, August 25, 2020 10:18 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander 
Subject: [PATCH] drm/amdgpu: report DC not supported if virtual display is 
enabled (v2)

virtual display is non-atomic so report false to avoid checking atomic state 
and other atomic things at runtime.

v2: squash into the sr-iov check

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 5a948edcc93c..8f37f9f99105 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2861,7 +2861,7 @@ bool amdgpu_device_asic_has_dc_support(enum amd_asic_type 
asic_type)
  */
 bool amdgpu_device_has_dc_support(struct amdgpu_device *adev)  {
-   if (amdgpu_sriov_vf(adev))
+   if (amdgpu_sriov_vf(adev) || adev->enable_virtual_display)
return false;
 
return amdgpu_device_asic_has_dc_support(adev->asic_type);
--
2.25.4

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cguchun.chen%40amd.com%7C5dd32cf703464c63e3ba08d84901b99a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637339619046540701&sdata=vnfKXyNtJVE10V6qFwzHQQ7rmVAV9Le6g31nkmaPnLA%3D&reserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH] drm/amdgpu: report DC not supported if virtual display is enabled (v2)

2020-08-25 Thread Alex Deucher
virtual display is non-atomic so report false to avoid checking
atomic state and other atomic things at runtime.

v2: squash into the sr-iov check

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 5a948edcc93c..8f37f9f99105 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2861,7 +2861,7 @@ bool amdgpu_device_asic_has_dc_support(enum amd_asic_type 
asic_type)
  */
 bool amdgpu_device_has_dc_support(struct amdgpu_device *adev)
 {
-   if (amdgpu_sriov_vf(adev))
+   if (amdgpu_sriov_vf(adev) || adev->enable_virtual_display)
return false;
 
return amdgpu_device_asic_has_dc_support(adev->asic_type);
-- 
2.25.4

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


[PATCH v2] drm/amdgpu: add support for user trap handlers

2020-08-25 Thread Samuel Pitoiset
A trap handler can be used by userspace to catch shader exceptions
like divide by zero, memory violations etc.

On GFX6-GFX8, the registers used to configure TBA/TMA aren't
privileged and can be configured from userpace.

On GFX9+ they are per VMID and privileged, only the KMD can
configure them. At the moment, we don't know how to set them
via the CP, so they are only emitted if a VMID is reserved.

This introduces a new CS chunk that can be used to set the
TBA/TMA virtual address at submit time.

TODO:
- rebase on top of amd-staging-drm-next (this branch currently
hangs my GPU at boot)

Signed-off-by: Samuel Pitoiset 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c   | 31 
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c  |  3 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h  |  4 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.h  |  4 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  4 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c   | 20 +--
 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c   | 20 +++
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c| 19 +++
 include/uapi/drm/amdgpu_drm.h|  8 ++
 9 files changed, 110 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index a512ccbc4dea..6ca5c4912e3a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -104,6 +104,19 @@ static int amdgpu_cs_bo_handles_chunk(struct 
amdgpu_cs_parser *p,
return r;
 }
 
+static int amdgpu_cs_user_trap_chunk(struct amdgpu_cs_parser *p,
+struct drm_amdgpu_cs_chunk_trap *data,
+uint64_t *tba_addr, uint64_t *tma_addr)
+{
+   if (!data->tba_addr || !data->tma_addr)
+   return -EINVAL;
+
+   *tba_addr = data->tba_addr;
+   *tma_addr = data->tma_addr;
+
+   return 0;
+}
+
 static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, union 
drm_amdgpu_cs *cs)
 {
struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
@@ -112,6 +125,7 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser 
*p, union drm_amdgpu_cs
uint64_t *chunk_array;
unsigned size, num_ibs = 0;
uint32_t uf_offset = 0;
+   uint64_t tba_addr = 0, tma_addr = 0;
int i;
int ret;
 
@@ -214,6 +228,19 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser 
*p, union drm_amdgpu_cs
 
break;
 
+   case AMDGPU_CHUNK_ID_TRAP:
+   size = sizeof(struct drm_amdgpu_cs_chunk_trap);
+   if (p->chunks[i].length_dw * sizeof(uint32_t) < size) {
+   ret = -EINVAL;
+   goto free_partial_kdata;
+   }
+
+   ret = amdgpu_cs_user_trap_chunk(p, p->chunks[i].kdata,
+   &tba_addr, &tma_addr);
+   if (ret)
+   goto free_partial_kdata;
+   break;
+
case AMDGPU_CHUNK_ID_DEPENDENCIES:
case AMDGPU_CHUNK_ID_SYNCOBJ_IN:
case AMDGPU_CHUNK_ID_SYNCOBJ_OUT:
@@ -239,6 +266,10 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser 
*p, union drm_amdgpu_cs
 
if (p->uf_entry.tv.bo)
p->job->uf_addr = uf_offset;
+
+   p->job->tba_addr = tba_addr;
+   p->job->tma_addr = tma_addr;
+
kfree(chunk_array);
 
/* Use this opportunity to fill in task info for the vm */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 26127c7d2f32..1e703119e4c2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -88,9 +88,10 @@
  * - 3.37.0 - L2 is invalidated before SDMA IBs, needed for correctness
  * - 3.38.0 - Add AMDGPU_IB_FLAG_EMIT_MEM_SYNC
  * - 3.39.0 - DMABUF implicit sync does a full pipeline sync
+ * - 3.40.0 - Add AMDGPU_CHUNK_ID_TRAP
  */
 #define KMS_DRIVER_MAJOR   3
-#define KMS_DRIVER_MINOR   39
+#define KMS_DRIVER_MINOR   40
 #define KMS_DRIVER_PATCHLEVEL  0
 
 int amdgpu_vram_limit = 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
index 8e58325bbca2..fd0d56724b4d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
@@ -58,6 +58,10 @@ struct amdgpu_vmid {
uint32_toa_base;
uint32_toa_size;
 
+   /* user trap */
+   uint64_ttba_addr;
+   uint64_ttma_addr;
+
unsignedpasid;
struct dma_fence*pasid_mapping;
 };
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
index 81caac9b958a..b8ed5b13ea44 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h

Re: [PATCH] drm/amdgpu: report DC not supported if virtual display is enabled

2020-08-25 Thread Deucher, Alexander
[AMD Public Use]

I can do that.

Alex


From: Chen, Guchun 
Sent: Tuesday, August 25, 2020 10:13 AM
To: Alex Deucher ; amd-gfx@lists.freedesktop.org 

Cc: Deucher, Alexander 
Subject: RE: [PATCH] drm/amdgpu: report DC not supported if virtual display is 
enabled

[AMD Public Use]

Why not merging it to the same line of SRIOV check?

if (amdgpu_sriov_vf(adev) || adev->enable_virtual_display )
return false;

Regards,
Guchun

-Original Message-
From: amd-gfx  On Behalf Of Alex Deucher
Sent: Tuesday, August 25, 2020 9:45 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander 
Subject: [PATCH] drm/amdgpu: report DC not supported if virtual display is 
enabled

virtual display is non-atomic so report false to avoid checking atomic state 
and other atomic things at runtime.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 5a948edcc93c..19f1e8449986 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2864,6 +2864,9 @@ bool amdgpu_device_has_dc_support(struct amdgpu_device 
*adev)
 if (amdgpu_sriov_vf(adev))
 return false;

+   if (adev->enable_virtual_display)
+   return false;
+
 return amdgpu_device_asic_has_dc_support(adev->asic_type);
 }

--
2.25.4

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cguchun.chen%40amd.com%7C6679ea962a8a4b5715cf08d848fd2469%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637339599368934081&sdata=WMMrhLJ2qXKEr6gHMn0ydF2%2B3jxzUEZ3azRsjkgOXZI%3D&reserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


RE: [PATCH] drm/amdgpu: report DC not supported if virtual display is enabled

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

Why not merging it to the same line of SRIOV check?

if (amdgpu_sriov_vf(adev) || adev->enable_virtual_display )
return false;

Regards,
Guchun

-Original Message-
From: amd-gfx  On Behalf Of Alex Deucher
Sent: Tuesday, August 25, 2020 9:45 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander 
Subject: [PATCH] drm/amdgpu: report DC not supported if virtual display is 
enabled

virtual display is non-atomic so report false to avoid checking atomic state 
and other atomic things at runtime.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 5a948edcc93c..19f1e8449986 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2864,6 +2864,9 @@ bool amdgpu_device_has_dc_support(struct amdgpu_device 
*adev)
if (amdgpu_sriov_vf(adev))
return false;
 
+   if (adev->enable_virtual_display)
+   return false;
+
return amdgpu_device_asic_has_dc_support(adev->asic_type);
 }
 
--
2.25.4

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cguchun.chen%40amd.com%7C6679ea962a8a4b5715cf08d848fd2469%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637339599368934081&sdata=WMMrhLJ2qXKEr6gHMn0ydF2%2B3jxzUEZ3azRsjkgOXZI%3D&reserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH] drm/amdgpu: report DC not supported if virtual display is enabled

2020-08-25 Thread Alex Deucher
virtual display is non-atomic so report false to avoid checking
atomic state and other atomic things at runtime.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 5a948edcc93c..19f1e8449986 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2864,6 +2864,9 @@ bool amdgpu_device_has_dc_support(struct amdgpu_device 
*adev)
if (amdgpu_sriov_vf(adev))
return false;
 
+   if (adev->enable_virtual_display)
+   return false;
+
return amdgpu_device_asic_has_dc_support(adev->asic_type);
 }
 
-- 
2.25.4

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


Re: [RFC PATCH] drm/amdgpu: add support for user trap handlers

2020-08-25 Thread Alex Deucher
On Tue, Aug 25, 2020 at 3:06 AM Samuel Pitoiset
 wrote:
>
>
> On 8/24/20 11:32 PM, Alex Deucher wrote:
> > On Mon, Aug 24, 2020 at 2:33 PM Alex Deucher  wrote:
> >> On Mon, Aug 24, 2020 at 7:57 AM Samuel Pitoiset
> >>  wrote:
> >>> A trap handler can be used by userspace to catch shader exceptions
> >>> like divide by zero, memory violations etc.
> >>>
> >>> On GFX6-GFX8, the registers used to configure TBA/TMA aren't
> >>> privileged while on GFX9+ they are per VMID and privileged,
> >>> so that only the KMD can configure them.
> >>>
> >>> This introduces a new CS chunk that can be used to set the
> >>> TBA/TMA virtual address at submit time.
> >>>
> >>> TODO:
> >>> - add GFX 6,7 and 10 support
> >>> - rebase on top of amd-staging-drm-next (this branch currently
> >>> hangs my GPU at boot)
> >>>
> >>> Signed-off-by: Samuel Pitoiset 
> >>> ---
> >>>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c   | 31 +
> >>>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c  |  3 +-
> >>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h  |  4 +++
> >>>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.h  |  4 +++
> >>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  4 +++
> >>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c   | 15 -
> >>>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c| 42 ++--
> >>>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c| 19 +++
> >>>   include/uapi/drm/amdgpu_drm.h|  8 +
> >>>   9 files changed, 126 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
> >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> >>> index a512ccbc4dea..6ca5c4912e3a 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> >>> @@ -104,6 +104,19 @@ static int amdgpu_cs_bo_handles_chunk(struct 
> >>> amdgpu_cs_parser *p,
> >>>  return r;
> >>>   }
> >>>
> >>> +static int amdgpu_cs_user_trap_chunk(struct amdgpu_cs_parser *p,
> >>> +struct drm_amdgpu_cs_chunk_trap 
> >>> *data,
> >>> +uint64_t *tba_addr, uint64_t 
> >>> *tma_addr)
> >>> +{
> >>> +   if (!data->tba_addr || !data->tma_addr)
> >>> +   return -EINVAL;
> >>> +
> >>> +   *tba_addr = data->tba_addr;
> >>> +   *tma_addr = data->tma_addr;
> >>> +
> >>> +   return 0;
> >>> +}
> >>> +
> >>>   static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, union 
> >>> drm_amdgpu_cs *cs)
> >>>   {
> >>>  struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
> >>> @@ -112,6 +125,7 @@ static int amdgpu_cs_parser_init(struct 
> >>> amdgpu_cs_parser *p, union drm_amdgpu_cs
> >>>  uint64_t *chunk_array;
> >>>  unsigned size, num_ibs = 0;
> >>>  uint32_t uf_offset = 0;
> >>> +   uint64_t tba_addr = 0, tma_addr = 0;
> >>>  int i;
> >>>  int ret;
> >>>
> >>> @@ -214,6 +228,19 @@ static int amdgpu_cs_parser_init(struct 
> >>> amdgpu_cs_parser *p, union drm_amdgpu_cs
> >>>
> >>>  break;
> >>>
> >>> +   case AMDGPU_CHUNK_ID_TRAP:
> >>> +   size = sizeof(struct drm_amdgpu_cs_chunk_trap);
> >>> +   if (p->chunks[i].length_dw * sizeof(uint32_t) < 
> >>> size) {
> >>> +   ret = -EINVAL;
> >>> +   goto free_partial_kdata;
> >>> +   }
> >>> +
> >>> +   ret = amdgpu_cs_user_trap_chunk(p, 
> >>> p->chunks[i].kdata,
> >>> +   &tba_addr, 
> >>> &tma_addr);
> >>> +   if (ret)
> >>> +   goto free_partial_kdata;
> >>> +   break;
> >>> +
> >>>  case AMDGPU_CHUNK_ID_DEPENDENCIES:
> >>>  case AMDGPU_CHUNK_ID_SYNCOBJ_IN:
> >>>  case AMDGPU_CHUNK_ID_SYNCOBJ_OUT:
> >>> @@ -239,6 +266,10 @@ static int amdgpu_cs_parser_init(struct 
> >>> amdgpu_cs_parser *p, union drm_amdgpu_cs
> >>>
> >>>  if (p->uf_entry.tv.bo)
> >>>  p->job->uf_addr = uf_offset;
> >>> +
> >>> +   p->job->tba_addr = tba_addr;
> >>> +   p->job->tma_addr = tma_addr;
> >>> +
> >>>  kfree(chunk_array);
> >>>
> >>>  /* Use this opportunity to fill in task info for the vm */
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >>> index 26127c7d2f32..1e703119e4c2 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >>> @@ -88,9 +88,10 @@
> >>>* - 3.37.0 - L2 is invalidated before SDMA IBs, needed for correctness
> >>>* - 3.38.0 - Add AMDGPU_IB_FLAG_EMIT_MEM_SYNC
> >>>* - 3.39.0 - DMABUF implicit sync does a full pipeline sync
> >>> + * - 3.40.0 - Add AMDGPU_CHUNK_ID_TRAP
> >>>*/
> >>>   #define KMS_DRIVER_MAJOR   3
> >>> -#define KMS_DRIVER_MINOR   39
> >>> 

[PATCH 3/3] drm/amdgpu/vi: fix buffer overflow in vi_get_register_value()

2020-08-25 Thread Dan Carpenter
The values for "se_num" and "sh_num" come from the user in the ioctl.
They can be in the 0-255 range but if they're more than
AMDGPU_GFX_MAX_SE (4) or AMDGPU_GFX_MAX_SH_PER_SE (2) then it results in
an out of bounds read.

I split this function into to two to make the error handling simpler.

Fixes: db9635cc14f3 ("drm/amdgpu: used cached gca values for vi_read_register 
(v2)")
Signed-off-by: Dan Carpenter 
---
 drivers/gpu/drm/amd/amdgpu/vi.c | 197 +---
 1 file changed, 105 insertions(+), 92 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c b/drivers/gpu/drm/amd/amdgpu/vi.c
index f6f2ed0830b1..fd623ad9d51f 100644
--- a/drivers/gpu/drm/amd/amdgpu/vi.c
+++ b/drivers/gpu/drm/amd/amdgpu/vi.c
@@ -527,99 +527,108 @@ static const struct amdgpu_allowed_register_entry 
vi_allowed_read_registers[] =
{mmPA_SC_RASTER_CONFIG_1, true},
 };
 
-static uint32_t vi_get_register_value(struct amdgpu_device *adev,
- bool indexed, u32 se_num,
- u32 sh_num, u32 reg_offset)
-{
-   if (indexed) {
-   uint32_t val;
-   unsigned se_idx = (se_num == 0x) ? 0 : se_num;
-   unsigned sh_idx = (sh_num == 0x) ? 0 : sh_num;
-
-   switch (reg_offset) {
-   case mmCC_RB_BACKEND_DISABLE:
-   return 
adev->gfx.config.rb_config[se_idx][sh_idx].rb_backend_disable;
-   case mmGC_USER_RB_BACKEND_DISABLE:
-   return 
adev->gfx.config.rb_config[se_idx][sh_idx].user_rb_backend_disable;
-   case mmPA_SC_RASTER_CONFIG:
-   return 
adev->gfx.config.rb_config[se_idx][sh_idx].raster_config;
-   case mmPA_SC_RASTER_CONFIG_1:
-   return 
adev->gfx.config.rb_config[se_idx][sh_idx].raster_config_1;
-   }
+static int vi_get_register_value_indexed(struct amdgpu_device *adev,
+u32 se_num, u32 sh_num,
+u32 reg_offset, u32 *value)
+{
+   unsigned se_idx = (se_num == 0x) ? 0 : se_num;
+   unsigned sh_idx = (sh_num == 0x) ? 0 : sh_num;
 
-   mutex_lock(&adev->grbm_idx_mutex);
-   if (se_num != 0x || sh_num != 0x)
-   amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 
0x);
+   if (se_idx >= AMDGPU_GFX_MAX_SE ||
+   sh_idx >= AMDGPU_GFX_MAX_SH_PER_SE)
+   return -EINVAL;
 
-   val = RREG32(reg_offset);
+   switch (reg_offset) {
+   case mmCC_RB_BACKEND_DISABLE:
+   *value = 
adev->gfx.config.rb_config[se_idx][sh_idx].rb_backend_disable;
+   return 0;
+   case mmGC_USER_RB_BACKEND_DISABLE:
+   *value = 
adev->gfx.config.rb_config[se_idx][sh_idx].user_rb_backend_disable;
+   return 0;
+   case mmPA_SC_RASTER_CONFIG:
+   *value = 
adev->gfx.config.rb_config[se_idx][sh_idx].raster_config;
+   return 0;
+   case mmPA_SC_RASTER_CONFIG_1:
+   *value = 
adev->gfx.config.rb_config[se_idx][sh_idx].raster_config_1;
+   return 0;
+   }
 
-   if (se_num != 0x || sh_num != 0x)
-   amdgpu_gfx_select_se_sh(adev, 0x, 0x, 
0x);
-   mutex_unlock(&adev->grbm_idx_mutex);
-   return val;
-   } else {
-   unsigned idx;
-
-   switch (reg_offset) {
-   case mmGB_ADDR_CONFIG:
-   return adev->gfx.config.gb_addr_config;
-   case mmMC_ARB_RAMCFG:
-   return adev->gfx.config.mc_arb_ramcfg;
-   case mmGB_TILE_MODE0:
-   case mmGB_TILE_MODE1:
-   case mmGB_TILE_MODE2:
-   case mmGB_TILE_MODE3:
-   case mmGB_TILE_MODE4:
-   case mmGB_TILE_MODE5:
-   case mmGB_TILE_MODE6:
-   case mmGB_TILE_MODE7:
-   case mmGB_TILE_MODE8:
-   case mmGB_TILE_MODE9:
-   case mmGB_TILE_MODE10:
-   case mmGB_TILE_MODE11:
-   case mmGB_TILE_MODE12:
-   case mmGB_TILE_MODE13:
-   case mmGB_TILE_MODE14:
-   case mmGB_TILE_MODE15:
-   case mmGB_TILE_MODE16:
-   case mmGB_TILE_MODE17:
-   case mmGB_TILE_MODE18:
-   case mmGB_TILE_MODE19:
-   case mmGB_TILE_MODE20:
-   case mmGB_TILE_MODE21:
-   case mmGB_TILE_MODE22:
-   case mmGB_TILE_MODE23:
-   case mmGB_TILE_MODE24:
-   case mmGB_TILE_MODE25:
-   case mmGB_TILE_MODE26:
-   case mmGB_TILE_MODE27:
-   case mmGB_TILE_MODE28:
-   case mmGB_TILE_MODE29:
-   case mmGB_TILE_MODE30:
-  

[PATCH 1/3] drm/amdgpu/si: Fix buffer overflow in si_get_register_value()

2020-08-25 Thread Dan Carpenter
The values for "se_num" and "sh_num" come from the user in the ioctl.
They can be in the 0-255 range but if they're more than
AMDGPU_GFX_MAX_SE (4) or AMDGPU_GFX_MAX_SH_PER_SE (2) then it results in
an out of bounds read.

I split this function into to two to make the error handling simpler.

Fixes: dd5dfa61b4ff ("drm/amdgpu: refine si_read_register")
Signed-off-by: Dan Carpenter 
---
 drivers/gpu/drm/amd/amdgpu/si.c | 157 +---
 1 file changed, 85 insertions(+), 72 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/si.c b/drivers/gpu/drm/amd/amdgpu/si.c
index e330884edd19..ccf39a6932ae 100644
--- a/drivers/gpu/drm/amd/amdgpu/si.c
+++ b/drivers/gpu/drm/amd/amdgpu/si.c
@@ -1051,81 +1051,90 @@ static struct amdgpu_allowed_register_entry 
si_allowed_read_registers[] = {
{PA_SC_RASTER_CONFIG, true},
 };
 
-static uint32_t si_get_register_value(struct amdgpu_device *adev,
- bool indexed, u32 se_num,
- u32 sh_num, u32 reg_offset)
-{
-   if (indexed) {
-   uint32_t val;
-   unsigned se_idx = (se_num == 0x) ? 0 : se_num;
-   unsigned sh_idx = (sh_num == 0x) ? 0 : sh_num;
-
-   switch (reg_offset) {
-   case mmCC_RB_BACKEND_DISABLE:
-   return 
adev->gfx.config.rb_config[se_idx][sh_idx].rb_backend_disable;
-   case mmGC_USER_RB_BACKEND_DISABLE:
-   return 
adev->gfx.config.rb_config[se_idx][sh_idx].user_rb_backend_disable;
-   case mmPA_SC_RASTER_CONFIG:
-   return 
adev->gfx.config.rb_config[se_idx][sh_idx].raster_config;
-   }
+static int si_get_register_value_indexed(struct amdgpu_device *adev,
+ u32 se_num, u32 sh_num,
+ u32 reg_offset, u32 *value)
+{
+   unsigned se_idx = (se_num == 0x) ? 0 : se_num;
+   unsigned sh_idx = (sh_num == 0x) ? 0 : sh_num;
 
-   mutex_lock(&adev->grbm_idx_mutex);
-   if (se_num != 0x || sh_num != 0x)
-   amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 
0x);
+   if (se_idx >= AMDGPU_GFX_MAX_SE ||
+   sh_idx >= AMDGPU_GFX_MAX_SH_PER_SE)
+   return -EINVAL;
 
-   val = RREG32(reg_offset);
+   switch (reg_offset) {
+   case mmCC_RB_BACKEND_DISABLE:
+   *value = 
adev->gfx.config.rb_config[se_idx][sh_idx].rb_backend_disable;
+   return 0;
+   case mmGC_USER_RB_BACKEND_DISABLE:
+   *value = 
adev->gfx.config.rb_config[se_idx][sh_idx].user_rb_backend_disable;
+   return 0;
+   case mmPA_SC_RASTER_CONFIG:
+   *value = 
adev->gfx.config.rb_config[se_idx][sh_idx].raster_config;
+   return 0;
+   }
 
-   if (se_num != 0x || sh_num != 0x)
-   amdgpu_gfx_select_se_sh(adev, 0x, 0x, 
0x);
-   mutex_unlock(&adev->grbm_idx_mutex);
-   return val;
-   } else {
-   unsigned idx;
-
-   switch (reg_offset) {
-   case mmGB_ADDR_CONFIG:
-   return adev->gfx.config.gb_addr_config;
-   case mmMC_ARB_RAMCFG:
-   return adev->gfx.config.mc_arb_ramcfg;
-   case mmGB_TILE_MODE0:
-   case mmGB_TILE_MODE1:
-   case mmGB_TILE_MODE2:
-   case mmGB_TILE_MODE3:
-   case mmGB_TILE_MODE4:
-   case mmGB_TILE_MODE5:
-   case mmGB_TILE_MODE6:
-   case mmGB_TILE_MODE7:
-   case mmGB_TILE_MODE8:
-   case mmGB_TILE_MODE9:
-   case mmGB_TILE_MODE10:
-   case mmGB_TILE_MODE11:
-   case mmGB_TILE_MODE12:
-   case mmGB_TILE_MODE13:
-   case mmGB_TILE_MODE14:
-   case mmGB_TILE_MODE15:
-   case mmGB_TILE_MODE16:
-   case mmGB_TILE_MODE17:
-   case mmGB_TILE_MODE18:
-   case mmGB_TILE_MODE19:
-   case mmGB_TILE_MODE20:
-   case mmGB_TILE_MODE21:
-   case mmGB_TILE_MODE22:
-   case mmGB_TILE_MODE23:
-   case mmGB_TILE_MODE24:
-   case mmGB_TILE_MODE25:
-   case mmGB_TILE_MODE26:
-   case mmGB_TILE_MODE27:
-   case mmGB_TILE_MODE28:
-   case mmGB_TILE_MODE29:
-   case mmGB_TILE_MODE30:
-   case mmGB_TILE_MODE31:
-   idx = (reg_offset - mmGB_TILE_MODE0);
-   return adev->gfx.config.tile_mode_array[idx];
-   default:
-   return RREG32(reg_offset);
-   }
+   mutex_lock(&adev->grbm_idx_mutex);
+   if (se_num != 0

[PATCH 2/3] drm/amdgpu/cik: fix buffer overflow in cik_get_register_value()

2020-08-25 Thread Dan Carpenter
The values for "se_num" and "sh_num" come from the user in the ioctl.
They can be in the 0-255 range but if they're more than
AMDGPU_GFX_MAX_SE (4) or AMDGPU_GFX_MAX_SH_PER_SE (2) then it results in
an out of bounds read.

I split this function into to two to make the error handling simpler.

Fixes: aca31681b1a5 ("drm/amdgpu: used cached gca values for cik_read_register")
Signed-off-by: Dan Carpenter 
---
 drivers/gpu/drm/amd/amdgpu/cik.c | 195 ---
 1 file changed, 103 insertions(+), 92 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c b/drivers/gpu/drm/amd/amdgpu/cik.c
index 7e71ffbca93d..91ebfa485333 100644
--- a/drivers/gpu/drm/amd/amdgpu/cik.c
+++ b/drivers/gpu/drm/amd/amdgpu/cik.c
@@ -1042,100 +1042,108 @@ static const struct amdgpu_allowed_register_entry 
cik_allowed_read_registers[] =
{mmPA_SC_RASTER_CONFIG_1, true},
 };
 
+static int cik_get_register_value_indexed(struct amdgpu_device *adev,
+ u32 se_num, u32 sh_num,
+ u32 reg_offset, u32 *value)
+{
+   unsigned se_idx = (se_num == 0x) ? 0 : se_num;
+   unsigned sh_idx = (sh_num == 0x) ? 0 : sh_num;
 
-static uint32_t cik_get_register_value(struct amdgpu_device *adev,
-  bool indexed, u32 se_num,
-  u32 sh_num, u32 reg_offset)
-{
-   if (indexed) {
-   uint32_t val;
-   unsigned se_idx = (se_num == 0x) ? 0 : se_num;
-   unsigned sh_idx = (sh_num == 0x) ? 0 : sh_num;
-
-   switch (reg_offset) {
-   case mmCC_RB_BACKEND_DISABLE:
-   return 
adev->gfx.config.rb_config[se_idx][sh_idx].rb_backend_disable;
-   case mmGC_USER_RB_BACKEND_DISABLE:
-   return 
adev->gfx.config.rb_config[se_idx][sh_idx].user_rb_backend_disable;
-   case mmPA_SC_RASTER_CONFIG:
-   return 
adev->gfx.config.rb_config[se_idx][sh_idx].raster_config;
-   case mmPA_SC_RASTER_CONFIG_1:
-   return 
adev->gfx.config.rb_config[se_idx][sh_idx].raster_config_1;
-   }
+   if (se_idx >= AMDGPU_GFX_MAX_SE ||
+   sh_idx >= AMDGPU_GFX_MAX_SH_PER_SE)
+   return -EINVAL;
+
+   switch (reg_offset) {
+   case mmCC_RB_BACKEND_DISABLE:
+   *value = 
adev->gfx.config.rb_config[se_idx][sh_idx].rb_backend_disable;
+   return 0;
+   case mmGC_USER_RB_BACKEND_DISABLE:
+   *value = 
adev->gfx.config.rb_config[se_idx][sh_idx].user_rb_backend_disable;
+   return 0;
+   case mmPA_SC_RASTER_CONFIG:
+   *value = 
adev->gfx.config.rb_config[se_idx][sh_idx].raster_config;
+   return 0;
+   case mmPA_SC_RASTER_CONFIG_1:
+   *value = 
adev->gfx.config.rb_config[se_idx][sh_idx].raster_config_1;
+   return 0;
+   }
 
-   mutex_lock(&adev->grbm_idx_mutex);
-   if (se_num != 0x || sh_num != 0x)
-   amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 
0x);
+   mutex_lock(&adev->grbm_idx_mutex);
+   if (se_num != 0x || sh_num != 0x)
+   amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 0x);
 
-   val = RREG32(reg_offset);
+   *value = RREG32(reg_offset);
 
-   if (se_num != 0x || sh_num != 0x)
-   amdgpu_gfx_select_se_sh(adev, 0x, 0x, 
0x);
-   mutex_unlock(&adev->grbm_idx_mutex);
-   return val;
-   } else {
-   unsigned idx;
-
-   switch (reg_offset) {
-   case mmGB_ADDR_CONFIG:
-   return adev->gfx.config.gb_addr_config;
-   case mmMC_ARB_RAMCFG:
-   return adev->gfx.config.mc_arb_ramcfg;
-   case mmGB_TILE_MODE0:
-   case mmGB_TILE_MODE1:
-   case mmGB_TILE_MODE2:
-   case mmGB_TILE_MODE3:
-   case mmGB_TILE_MODE4:
-   case mmGB_TILE_MODE5:
-   case mmGB_TILE_MODE6:
-   case mmGB_TILE_MODE7:
-   case mmGB_TILE_MODE8:
-   case mmGB_TILE_MODE9:
-   case mmGB_TILE_MODE10:
-   case mmGB_TILE_MODE11:
-   case mmGB_TILE_MODE12:
-   case mmGB_TILE_MODE13:
-   case mmGB_TILE_MODE14:
-   case mmGB_TILE_MODE15:
-   case mmGB_TILE_MODE16:
-   case mmGB_TILE_MODE17:
-   case mmGB_TILE_MODE18:
-   case mmGB_TILE_MODE19:
-   case mmGB_TILE_MODE20:
-   case mmGB_TILE_MODE21:
-   case mmGB_TILE_MODE22:
-   case mmGB_TILE_MODE23:
-   case mmGB_TILE_MODE24:
-   cas

[bug report] drm/amd/powerplay: add point check to avoid NULL point hang.

2020-08-25 Thread Dan Carpenter
Hello Rex Zhu,

The patch 88b8dcbe21fd: "drm/amd/powerplay: add point check to avoid
NULL point hang." from Dec 11, 2015, leads to the following static
checker warning:

drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/hardwaremanager.c:274 
phm_check_smc_update_required_for_display_configuration()
warn: signedness bug returning '(-22)'

drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/hardwaremanager.c
   272  bool phm_check_smc_update_required_for_display_configuration(struct 
pp_hwmgr *hwmgr)
   273  {
   274  PHM_FUNC_CHECK(hwmgr);

PHM_FUNC_CHECK() has a hiddren -EINVAL return that becomes true when
casted to bool.

   275  if (hwmgr->pp_one_vf)
   276  return false;
   277  
   278  if 
(hwmgr->hwmgr_func->check_smc_update_required_for_display_configuration == NULL)
   279  return false;
   280  
   281  return 
hwmgr->hwmgr_func->check_smc_update_required_for_display_configuration(hwmgr);
   282  }

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


RE: [PATCH] drm/amdgpu: use MODE1 reset for navy_flounder by default

2020-08-25 Thread Zhou1, Tao
[AMD Public Use]

Reviewed-by: Tao Zhou 

> -Original Message-
> From: Jiansong Chen 
> Sent: Tuesday, August 25, 2020 3:59 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Zhou1, Tao ; Chen, Jiansong (Simon)
> 
> Subject: [PATCH] drm/amdgpu: use MODE1 reset for navy_flounder by default
> 
> Switch default gpu reset method to MODE1 for navy_flounder.
> 
> Signed-off-by: Jiansong Chen 
> Change-Id: I99b2d3ac04352142e288877f3b6c3138d0efd4bc
> ---
>  drivers/gpu/drm/amd/amdgpu/nv.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c
> b/drivers/gpu/drm/amd/amdgpu/nv.c index 33a6d2d5fc16..4d1402356262
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> @@ -364,6 +364,7 @@ nv_asic_reset_method(struct amdgpu_device *adev)
> 
>   switch (adev->asic_type) {
>   case CHIP_SIENNA_CICHLID:
> + case CHIP_NAVY_FLOUNDER:
>   return AMD_RESET_METHOD_MODE1;
>   default:
>   if (smu_baco_is_support(smu))
> --
> 2.25.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH] drm/amdgpu: use MODE1 reset for navy_flounder by default

2020-08-25 Thread Jiansong Chen
Switch default gpu reset method to MODE1 for navy_flounder.

Signed-off-by: Jiansong Chen 
Change-Id: I99b2d3ac04352142e288877f3b6c3138d0efd4bc
---
 drivers/gpu/drm/amd/amdgpu/nv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
index 33a6d2d5fc16..4d1402356262 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -364,6 +364,7 @@ nv_asic_reset_method(struct amdgpu_device *adev)
 
switch (adev->asic_type) {
case CHIP_SIENNA_CICHLID:
+   case CHIP_NAVY_FLOUNDER:
return AMD_RESET_METHOD_MODE1;
default:
if (smu_baco_is_support(smu))
-- 
2.25.1

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


[PATCH 1/4] drm/amd/pm: drop unnecessary feature->mutex lock protections(V2)

2020-08-25 Thread Evan Quan
As these operations are performed in hardware setup and there
is actually no race conditions during this period considering:
1. the hardware setup is serial and cannnot be in parallel
2. all other operations can be performed only after hardware
   setup complete.

V2: rich the commit log description

Change-Id: I096d7ab0855ff59b0ecb56fd9d6d9946b3605fc8
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c  | 4 
 drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 2 --
 2 files changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c 
b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 09dc5303762b..b7cad8ef6153 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -361,20 +361,16 @@ static int smu_get_driver_allowed_feature_mask(struct 
smu_context *smu)
int ret = 0;
uint32_t allowed_feature_mask[SMU_FEATURE_MAX/32];
 
-   mutex_lock(&feature->mutex);
bitmap_zero(feature->allowed, SMU_FEATURE_MAX);
-   mutex_unlock(&feature->mutex);
 
ret = smu_get_allowed_feature_mask(smu, allowed_feature_mask,
 SMU_FEATURE_MAX/32);
if (ret)
return ret;
 
-   mutex_lock(&feature->mutex);
bitmap_or(feature->allowed, feature->allowed,
  (unsigned long *)allowed_feature_mask,
  feature->feature_num);
-   mutex_unlock(&feature->mutex);
 
return ret;
 }
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
index 548db1edd352..28a19ffd22a1 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
@@ -721,7 +721,6 @@ int smu_v11_0_set_allowed_mask(struct smu_context *smu)
int ret = 0;
uint32_t feature_mask[2];
 
-   mutex_lock(&feature->mutex);
if (bitmap_empty(feature->allowed, SMU_FEATURE_MAX) || 
feature->feature_num < 64)
goto failed;
 
@@ -738,7 +737,6 @@ int smu_v11_0_set_allowed_mask(struct smu_context *smu)
goto failed;
 
 failed:
-   mutex_unlock(&feature->mutex);
return ret;
 }
 
-- 
2.28.0

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


[PATCH 4/4] drm/amd/pm: minor cleanups

2020-08-25 Thread Evan Quan
Drop unneeded "ret".

Change-Id: If5eabb1e96153133a833d0e5b1dca9c0f0928891
Signed-off-by: Evan Quan 
Reviewed-by: Alex Deucher 
---
 .../gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c| 22 +--
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
index a83b3635df40..e4d333d1b9ca 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
@@ -1172,12 +1172,10 @@ int smu_v11_0_set_fan_speed_rpm(struct smu_context *smu,
 int smu_v11_0_set_xgmi_pstate(struct smu_context *smu,
 uint32_t pstate)
 {
-   int ret = 0;
-   ret = smu_cmn_send_smc_msg_with_param(smu,
- SMU_MSG_SetXgmiMode,
- pstate ? XGMI_MODE_PSTATE_D0 : 
XGMI_MODE_PSTATE_D3,
+   return smu_cmn_send_smc_msg_with_param(smu,
+  SMU_MSG_SetXgmiMode,
+  pstate ? XGMI_MODE_PSTATE_D0 : 
XGMI_MODE_PSTATE_D3,
  NULL);
-   return ret;
 }
 
 static int smu_v11_0_set_irq_state(struct amdgpu_device *adev,
@@ -1408,11 +1406,7 @@ int smu_v11_0_get_max_sustainable_clocks_by_dc(struct 
smu_context *smu,
 
 int smu_v11_0_set_azalia_d3_pme(struct smu_context *smu)
 {
-   int ret = 0;
-
-   ret = smu_cmn_send_smc_msg(smu, SMU_MSG_BacoAudioD3PME, NULL);
-
-   return ret;
+   return smu_cmn_send_smc_msg(smu, SMU_MSG_BacoAudioD3PME, NULL);
 }
 
 static int smu_v11_0_baco_set_armd3_sequence(struct smu_context *smu, enum 
smu_v11_0_baco_seq baco_seq)
@@ -1511,13 +1505,7 @@ int smu_v11_0_baco_enter(struct smu_context *smu)
 
 int smu_v11_0_baco_exit(struct smu_context *smu)
 {
-   int ret = 0;
-
-   ret = smu_v11_0_baco_set_state(smu, SMU_BACO_STATE_EXIT);
-   if (ret)
-   return ret;
-
-   return ret;
+   return smu_v11_0_baco_set_state(smu, SMU_BACO_STATE_EXIT);
 }
 
 int smu_v11_0_mode1_reset(struct smu_context *smu)
-- 
2.28.0

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


[PATCH 3/4] drm/amd/pm: drop unnecessary table existence and dpm enablement check

2020-08-25 Thread Evan Quan
Either this was already performed in parent API. Or the table is
confirmed to exist.

Change-Id: Ie6778a5035749221e0f9d5ad977a0e56392771dd
Signed-off-by: Evan Quan 
Reviewed-by: Alex Deucher 
---
 drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h   |  1 -
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 16 
 .../gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c| 19 ---
 drivers/gpu/drm/amd/pm/swsmu/smu_internal.h   |  1 +
 4 files changed, 5 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h 
b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
index 4c5c041af4ee..1888776deccb 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
@@ -701,7 +701,6 @@ int smu_set_fan_speed_percent(struct smu_context *smu, 
uint32_t speed);
 int smu_get_fan_speed_rpm(struct smu_context *smu, uint32_t *speed);
 
 int smu_set_deep_sleep_dcefclk(struct smu_context *smu, int clk);
-int smu_set_active_display_count(struct smu_context *smu, uint32_t count);
 
 int smu_get_clock_by_type(struct smu_context *smu,
  enum amd_pp_clock_type type,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c 
b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index b7cad8ef6153..8a42d976a930 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -572,9 +572,6 @@ static int smu_fini_fb_allocations(struct smu_context *smu)
struct smu_table *tables = smu_table->tables;
struct smu_table *driver_table = &(smu_table->driver_table);
 
-   if (!tables)
-   return 0;
-
if (tables[SMU_TABLE_PMSTATUSLOG].mc_address)
amdgpu_bo_free_kernel(&tables[SMU_TABLE_PMSTATUSLOG].bo,
  &tables[SMU_TABLE_PMSTATUSLOG].mc_address,
@@ -2252,19 +2249,6 @@ int smu_set_deep_sleep_dcefclk(struct smu_context *smu, 
int clk)
return ret;
 }
 
-int smu_set_active_display_count(struct smu_context *smu, uint32_t count)
-{
-   int ret = 0;
-
-   if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)
-   return -EOPNOTSUPP;
-
-   if (smu->ppt_funcs->set_active_display_count)
-   ret = smu->ppt_funcs->set_active_display_count(smu, count);
-
-   return ret;
-}
-
 int smu_get_clock_by_type(struct smu_context *smu,
  enum amd_pp_clock_type type,
  struct amd_pp_clocks *clocks)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
index 005673bedc2f..a83b3635df40 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
@@ -453,9 +453,6 @@ int smu_v11_0_init_power(struct smu_context *smu)
 {
struct smu_power_context *smu_power = &smu->smu_power;
 
-   if (smu_power->power_context || smu_power->power_context_size != 0)
-   return -EINVAL;
-
smu_power->power_context = kzalloc(sizeof(struct smu_11_0_dpm_context),
   GFP_KERNEL);
if (!smu_power->power_context)
@@ -469,9 +466,6 @@ int smu_v11_0_fini_power(struct smu_context *smu)
 {
struct smu_power_context *smu_power = &smu->smu_power;
 
-   if (!smu_power->power_context || smu_power->power_context_size == 0)
-   return -EINVAL;
-
kfree(smu_power->power_context);
smu_power->power_context = NULL;
smu_power->power_context_size = 0;
@@ -700,18 +694,16 @@ int smu_v11_0_set_tool_table_location(struct smu_context 
*smu)
 
 int smu_v11_0_init_display_count(struct smu_context *smu, uint32_t count)
 {
-   int ret = 0;
struct amdgpu_device *adev = smu->adev;
 
/* Navy_Flounder do not support to change display num currently */
if (adev->asic_type == CHIP_NAVY_FLOUNDER)
return 0;
 
-   if (!smu->pm_enabled)
-   return ret;
-
-   ret = smu_cmn_send_smc_msg_with_param(smu, SMU_MSG_NumOfDisplays, 
count, NULL);
-   return ret;
+   return smu_cmn_send_smc_msg_with_param(smu,
+  SMU_MSG_NumOfDisplays,
+  count,
+  NULL);
 }
 
 
@@ -773,9 +765,6 @@ int smu_v11_0_notify_display_change(struct smu_context *smu)
 {
int ret = 0;
 
-   if (!smu->pm_enabled)
-   return ret;
-
if (smu_cmn_feature_is_enabled(smu, SMU_FEATURE_DPM_UCLK_BIT) &&
smu->adev->gmc.vram_type == AMDGPU_VRAM_TYPE_HBM)
ret = smu_cmn_send_smc_msg_with_param(smu, 
SMU_MSG_SetUclkFastSwitch, 1, NULL);
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h 
b/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
index c88f8fab1bae..5c8bb7314675 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
@@ -42,6 +42,7 @@
 #define smu_ch

[PATCH 2/4] drm/amd/pm: drop unnecessary smu_baco->mutex lock protections(V2)

2020-08-25 Thread Evan Quan
As these operations are performed in hardware setup and there
is actually no race conditions during this period considering:
1. the hardware setup is serial and cannnot be in parallel
2. all other operations can be performed only after hardware
   setup complete.

V2: rich the commit log description

Change-Id: I7078ac26ae71eb6c7cbf918a127adfc2f56acf7d
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c   | 2 --
 drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 2 --
 drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 2 --
 drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c  | 7 +--
 4 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
index 8347b1f2509f..4195b5b9e8e5 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
@@ -386,11 +386,9 @@ static int arcturus_check_powerplay_table(struct 
smu_context *smu)
table_context->power_play_table;
struct smu_baco_context *smu_baco = &smu->smu_baco;
 
-   mutex_lock(&smu_baco->mutex);
if (powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_BACO ||
powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_MACO)
smu_baco->platform_support = true;
-   mutex_unlock(&smu_baco->mutex);
 
table_context->thermal_controller_type =
powerplay_table->thermal_controller_type;
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index 72f3d68691d8..f4b26399b413 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -347,11 +347,9 @@ static int navi10_check_powerplay_table(struct smu_context 
*smu)
if (powerplay_table->platform_caps & 
SMU_11_0_PP_PLATFORM_CAP_HARDWAREDC)
smu->dc_controlled_by_gpio = true;
 
-   mutex_lock(&smu_baco->mutex);
if (powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_BACO ||
powerplay_table->platform_caps & SMU_11_0_PP_PLATFORM_CAP_MACO)
smu_baco->platform_support = true;
-   mutex_unlock(&smu_baco->mutex);
 
table_context->thermal_controller_type =
powerplay_table->thermal_controller_type;
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
index 66d655958a78..c19f526c5a22 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
@@ -296,11 +296,9 @@ static int sienna_cichlid_check_powerplay_table(struct 
smu_context *smu)
table_context->power_play_table;
struct smu_baco_context *smu_baco = &smu->smu_baco;
 
-   mutex_lock(&smu_baco->mutex);
if (powerplay_table->platform_caps & SMU_11_0_7_PP_PLATFORM_CAP_BACO ||
powerplay_table->platform_caps & SMU_11_0_7_PP_PLATFORM_CAP_MACO)
smu_baco->platform_support = true;
-   mutex_unlock(&smu_baco->mutex);
 
table_context->thermal_controller_type =
powerplay_table->thermal_controller_type;
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
index 28a19ffd22a1..005673bedc2f 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
@@ -1434,13 +1434,8 @@ static int smu_v11_0_baco_set_armd3_sequence(struct 
smu_context *smu, enum smu_v
 bool smu_v11_0_baco_is_support(struct smu_context *smu)
 {
struct smu_baco_context *smu_baco = &smu->smu_baco;
-   bool baco_support;
 
-   mutex_lock(&smu_baco->mutex);
-   baco_support = smu_baco->platform_support;
-   mutex_unlock(&smu_baco->mutex);
-
-   if (!baco_support)
+   if (!smu_baco->platform_support)
return false;
 
/* Arcturus does not support this bit mask */
-- 
2.28.0

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


Re: [RFC PATCH] drm/amdgpu: add support for user trap handlers

2020-08-25 Thread Samuel Pitoiset



On 8/24/20 11:32 PM, Alex Deucher wrote:

On Mon, Aug 24, 2020 at 2:33 PM Alex Deucher  wrote:

On Mon, Aug 24, 2020 at 7:57 AM Samuel Pitoiset
 wrote:

A trap handler can be used by userspace to catch shader exceptions
like divide by zero, memory violations etc.

On GFX6-GFX8, the registers used to configure TBA/TMA aren't
privileged while on GFX9+ they are per VMID and privileged,
so that only the KMD can configure them.

This introduces a new CS chunk that can be used to set the
TBA/TMA virtual address at submit time.

TODO:
- add GFX 6,7 and 10 support
- rebase on top of amd-staging-drm-next (this branch currently
hangs my GPU at boot)

Signed-off-by: Samuel Pitoiset 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c   | 31 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c  |  3 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h  |  4 +++
  drivers/gpu/drm/amd/amdgpu/amdgpu_job.h  |  4 +++
  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  4 +++
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c   | 15 -
  drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c| 42 ++--
  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c| 19 +++
  include/uapi/drm/amdgpu_drm.h|  8 +
  9 files changed, 126 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index a512ccbc4dea..6ca5c4912e3a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -104,6 +104,19 @@ static int amdgpu_cs_bo_handles_chunk(struct 
amdgpu_cs_parser *p,
 return r;
  }

+static int amdgpu_cs_user_trap_chunk(struct amdgpu_cs_parser *p,
+struct drm_amdgpu_cs_chunk_trap *data,
+uint64_t *tba_addr, uint64_t *tma_addr)
+{
+   if (!data->tba_addr || !data->tma_addr)
+   return -EINVAL;
+
+   *tba_addr = data->tba_addr;
+   *tma_addr = data->tma_addr;
+
+   return 0;
+}
+
  static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, union 
drm_amdgpu_cs *cs)
  {
 struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
@@ -112,6 +125,7 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser 
*p, union drm_amdgpu_cs
 uint64_t *chunk_array;
 unsigned size, num_ibs = 0;
 uint32_t uf_offset = 0;
+   uint64_t tba_addr = 0, tma_addr = 0;
 int i;
 int ret;

@@ -214,6 +228,19 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser 
*p, union drm_amdgpu_cs

 break;

+   case AMDGPU_CHUNK_ID_TRAP:
+   size = sizeof(struct drm_amdgpu_cs_chunk_trap);
+   if (p->chunks[i].length_dw * sizeof(uint32_t) < size) {
+   ret = -EINVAL;
+   goto free_partial_kdata;
+   }
+
+   ret = amdgpu_cs_user_trap_chunk(p, p->chunks[i].kdata,
+   &tba_addr, &tma_addr);
+   if (ret)
+   goto free_partial_kdata;
+   break;
+
 case AMDGPU_CHUNK_ID_DEPENDENCIES:
 case AMDGPU_CHUNK_ID_SYNCOBJ_IN:
 case AMDGPU_CHUNK_ID_SYNCOBJ_OUT:
@@ -239,6 +266,10 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser 
*p, union drm_amdgpu_cs

 if (p->uf_entry.tv.bo)
 p->job->uf_addr = uf_offset;
+
+   p->job->tba_addr = tba_addr;
+   p->job->tma_addr = tma_addr;
+
 kfree(chunk_array);

 /* Use this opportunity to fill in task info for the vm */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 26127c7d2f32..1e703119e4c2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -88,9 +88,10 @@
   * - 3.37.0 - L2 is invalidated before SDMA IBs, needed for correctness
   * - 3.38.0 - Add AMDGPU_IB_FLAG_EMIT_MEM_SYNC
   * - 3.39.0 - DMABUF implicit sync does a full pipeline sync
+ * - 3.40.0 - Add AMDGPU_CHUNK_ID_TRAP
   */
  #define KMS_DRIVER_MAJOR   3
-#define KMS_DRIVER_MINOR   39
+#define KMS_DRIVER_MINOR   40
  #define KMS_DRIVER_PATCHLEVEL  0

  int amdgpu_vram_limit = 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
index 8e58325bbca2..fd0d56724b4d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
@@ -58,6 +58,10 @@ struct amdgpu_vmid {
 uint32_toa_base;
 uint32_toa_size;

+   /* user trap */
+   uint64_ttba_addr;
+   uint64_ttma_addr;
+
 unsignedpasid;
 struct dma_fence*pasid_mapping;
  };
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
index 8

Re: [RFC PATCH] drm/amdgpu: add support for user trap handlers

2020-08-25 Thread Samuel Pitoiset


On 8/24/20 8:17 PM, Marek Olšák wrote:
SET_SH_REG won't work with CP register shadowing. You need to use 
WRITE_DATA or WREG32.

You are right, will fix.


Marek

On Mon, Aug 24, 2020 at 7:57 AM Samuel Pitoiset 
mailto:samuel.pitoi...@gmail.com>> wrote:


A trap handler can be used by userspace to catch shader exceptions
like divide by zero, memory violations etc.

On GFX6-GFX8, the registers used to configure TBA/TMA aren't
privileged while on GFX9+ they are per VMID and privileged,
so that only the KMD can configure them.

This introduces a new CS chunk that can be used to set the
TBA/TMA virtual address at submit time.

TODO:
- add GFX 6,7 and 10 support
- rebase on top of amd-staging-drm-next (this branch currently
hangs my GPU at boot)

Signed-off-by: Samuel Pitoiset mailto:samuel.pitoi...@gmail.com>>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c   | 31 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c  |  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h  |  4 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.h  |  4 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  4 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c   | 15 -
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c    | 42
++--
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c    | 19 +++
 include/uapi/drm/amdgpu_drm.h            |  8 +
 9 files changed, 126 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index a512ccbc4dea..6ca5c4912e3a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -104,6 +104,19 @@ static int amdgpu_cs_bo_handles_chunk(struct
amdgpu_cs_parser *p,
        return r;
 }

+static int amdgpu_cs_user_trap_chunk(struct amdgpu_cs_parser *p,
+                                    struct
drm_amdgpu_cs_chunk_trap *data,
+                                    uint64_t *tba_addr, uint64_t
*tma_addr)
+{
+       if (!data->tba_addr || !data->tma_addr)
+               return -EINVAL;
+
+       *tba_addr = data->tba_addr;
+       *tma_addr = data->tma_addr;
+
+       return 0;
+}
+
 static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p,
union drm_amdgpu_cs *cs)
 {
        struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
@@ -112,6 +125,7 @@ static int amdgpu_cs_parser_init(struct
amdgpu_cs_parser *p, union drm_amdgpu_cs
        uint64_t *chunk_array;
        unsigned size, num_ibs = 0;
        uint32_t uf_offset = 0;
+       uint64_t tba_addr = 0, tma_addr = 0;
        int i;
        int ret;

@@ -214,6 +228,19 @@ static int amdgpu_cs_parser_init(struct
amdgpu_cs_parser *p, union drm_amdgpu_cs

                        break;

+               case AMDGPU_CHUNK_ID_TRAP:
+                       size = sizeof(struct
drm_amdgpu_cs_chunk_trap);
+                       if (p->chunks[i].length_dw *
sizeof(uint32_t) < size) {
+                               ret = -EINVAL;
+                               goto free_partial_kdata;
+                       }
+
+                       ret = amdgpu_cs_user_trap_chunk(p,
p->chunks[i].kdata,
+  &tba_addr, &tma_addr);
+                       if (ret)
+                               goto free_partial_kdata;
+                       break;
+
                case AMDGPU_CHUNK_ID_DEPENDENCIES:
                case AMDGPU_CHUNK_ID_SYNCOBJ_IN:
                case AMDGPU_CHUNK_ID_SYNCOBJ_OUT:
@@ -239,6 +266,10 @@ static int amdgpu_cs_parser_init(struct
amdgpu_cs_parser *p, union drm_amdgpu_cs

        if (p->uf_entry.tv.bo )
                p->job->uf_addr = uf_offset;
+
+       p->job->tba_addr = tba_addr;
+       p->job->tma_addr = tma_addr;
+
        kfree(chunk_array);

        /* Use this opportunity to fill in task info for the vm */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 26127c7d2f32..1e703119e4c2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -88,9 +88,10 @@
  * - 3.37.0 - L2 is invalidated before SDMA IBs, needed for
correctness
  * - 3.38.0 - Add AMDGPU_IB_FLAG_EMIT_MEM_SYNC
  * - 3.39.0 - DMABUF implicit sync does a full pipeline sync
+ * - 3.40.0 - Add AMDGPU_CHUNK_ID_TRAP
  */
 #define KMS_DRIVER_MAJOR       3
-#define KMS_DRIVER_MINOR       39
+#define KMS_DRIVER_MINOR       40
 #define KMS_DRIVER_PATCHLEVEL  0

 int amdgpu_vram_limit = 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
index 8e58325bbca2..fd0d56724b4d 100644
--- a/dr