<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<meta content="text/html; charset=utf-8">
</head>
<body bgcolor="#FFFFFF">
The pg lock is needed to read pgfsm registers so you wouldn't normally take grbm too.
<div><br>
</div>
<div>Tom</div>
<div><br>
</div>
<div><br>
</div>
<div><br>
</div>
<div>-------- Original message --------</div>
<div>From: Christian König <deathsimple@vodafone.de> </div>
<div>Date: 2016-10-10 09:49 (GMT-05:00) </div>
<div>To: Tom St Denis <tstdenis82@gmail.com>, amd-gfx@lists.freedesktop.org </div>
<div>Cc: "StDenis, Tom" <Tom.StDenis@amd.com> </div>
<div>Subject: Re: [PATCH] drm/amd/amdgpu: Make debugfs write compliment read </div>
<div><br>
</div>
<div>
<div class="moz-cite-prefix">Would it hurt us to always take the looks (both the pm as well as grbm idx lock)?<br>
<br>
I don't think that would be much of an issue and I would have a better feeling with that.<br>
<br>
Christian.<br>
<br>
Am 10.10.2016 um 15:38 schrieb Tom St Denis:<br>
</div>
<blockquote type="cite">
<p dir="ltr">that's kind of the point. Its for debugging only. Also reads can already lock</p>
<br>
<div class="gmail_quote">
<div dir="ltr">On Mon, Oct 10, 2016, 09:25 Christian König <<a href="mailto:deathsimple@vodafone.de">deathsimple@vodafone.de</a>> wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex; border-left:1px #ccc solid; padding-left:1ex">
Mhm, that userspace controls if the lock is taken or not is a bit<br class="gmail_msg">
problematic, on the other hand only root could access that.<br class="gmail_msg">
<br class="gmail_msg">
Christian.<br class="gmail_msg">
<br class="gmail_msg">
Am 10.10.2016 um 14:51 schrieb Tom St Denis:<br class="gmail_msg">
> Add PG lock support as well as bank selection to<br class="gmail_msg">
> the MMIO write function.<br class="gmail_msg">
><br class="gmail_msg">
> Signed-off-by: Tom St Denis <<a href="mailto:tom.stdenis@amd.com" class="gmail_msg" target="_blank">tom.stdenis@amd.com</a>><br class="gmail_msg">
> ---<br class="gmail_msg">
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 43 ++++++++++++++++++++++++++++++<br class="gmail_msg">
> 1 file changed, 43 insertions(+)<br class="gmail_msg">
><br class="gmail_msg">
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c<br class="gmail_msg">
> index 04c4aee70452..a9d83c1d7a43 100644<br class="gmail_msg">
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c<br class="gmail_msg">
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c<br class="gmail_msg">
> @@ -2637,10 +2637,45 @@ static ssize_t amdgpu_debugfs_regs_write(struct file *f, const char __user *buf,<br class="gmail_msg">
> struct amdgpu_device *adev = f->f_inode->i_private;<br class="gmail_msg">
> ssize_t result = 0;<br class="gmail_msg">
> int r;<br class="gmail_msg">
> + bool pm_pg_lock, use_bank;<br class="gmail_msg">
> + unsigned instance_bank, sh_bank, se_bank;<br class="gmail_msg">
><br class="gmail_msg">
> if (size & 0x3 || *pos & 0x3)<br class="gmail_msg">
> return -EINVAL;<br class="gmail_msg">
><br class="gmail_msg">
> + /* are we reading registers for which a PG lock is necessary? */<br class="gmail_msg">
> + pm_pg_lock = (*pos >> 23) & 1;<br class="gmail_msg">
> +<br class="gmail_msg">
> + if (*pos & (1ULL << 62)) {<br class="gmail_msg">
> + se_bank = (*pos >> 24) & 0x3FF;<br class="gmail_msg">
> + sh_bank = (*pos >> 34) & 0x3FF;<br class="gmail_msg">
> + instance_bank = (*pos >> 44) & 0x3FF;<br class="gmail_msg">
> +<br class="gmail_msg">
> + if (se_bank == 0x3FF)<br class="gmail_msg">
> + se_bank = 0xFFFFFFFF;<br class="gmail_msg">
> + if (sh_bank == 0x3FF)<br class="gmail_msg">
> + sh_bank = 0xFFFFFFFF;<br class="gmail_msg">
> + if (instance_bank == 0x3FF)<br class="gmail_msg">
> + instance_bank = 0xFFFFFFFF;<br class="gmail_msg">
> + use_bank = 1;<br class="gmail_msg">
> + } else {<br class="gmail_msg">
> + use_bank = 0;<br class="gmail_msg">
> + }<br class="gmail_msg">
> +<br class="gmail_msg">
> + *pos &= 0x3FFFF;<br class="gmail_msg">
> +<br class="gmail_msg">
> + if (use_bank) {<br class="gmail_msg">
> + if ((sh_bank != 0xFFFFFFFF && sh_bank >= adev->gfx.config.max_sh_per_se) ||<br class="gmail_msg">
> + (se_bank != 0xFFFFFFFF && se_bank >= adev->gfx.config.max_shader_engines))<br class="gmail_msg">
> + return -EINVAL;<br class="gmail_msg">
> + mutex_lock(&adev->grbm_idx_mutex);<br class="gmail_msg">
> + amdgpu_gfx_select_se_sh(adev, se_bank,<br class="gmail_msg">
> + sh_bank, instance_bank);<br class="gmail_msg">
> + }<br class="gmail_msg">
> +<br class="gmail_msg">
> + if (pm_pg_lock)<br class="gmail_msg">
> + mutex_lock(&adev->pm.mutex);<br class="gmail_msg">
> +<br class="gmail_msg">
> while (size) {<br class="gmail_msg">
> uint32_t value;<br class="gmail_msg">
><br class="gmail_msg">
> @@ -2659,6 +2694,14 @@ static ssize_t amdgpu_debugfs_regs_write(struct file *f, const char __user *buf,<br class="gmail_msg">
> size -= 4;<br class="gmail_msg">
> }<br class="gmail_msg">
><br class="gmail_msg">
> + if (use_bank) {<br class="gmail_msg">
> + amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff);<br class="gmail_msg">
> + mutex_unlock(&adev->grbm_idx_mutex);<br class="gmail_msg">
> + }<br class="gmail_msg">
> +<br class="gmail_msg">
> + if (pm_pg_lock)<br class="gmail_msg">
> + mutex_unlock(&adev->pm.mutex);<br class="gmail_msg">
> +<br class="gmail_msg">
> return result;<br class="gmail_msg">
> }<br class="gmail_msg">
><br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
</blockquote>
</div>
</blockquote>
<p><br>
</p>
</div>
</body>
</html>