Would it hurt us to always take the looks (both the pm as well as grbm idx lock)? I don't think that would be much of an issue and I would have a better feeling with that. Christian. Am 10.10.2016 um 15:38 schrieb Tom St Denis: > > that's kind of the point. Its for debugging only. Also reads can > already lock > > > On Mon, Oct 10, 2016, 09:25 Christian König <deathsimple at vodafone.de > <mailto:deathsimple at vodafone.de>> wrote: > > Mhm, that userspace controls if the lock is taken or not is a bit > problematic, on the other hand only root could access that. > > Christian. > > Am 10.10.2016 um 14:51 schrieb Tom St Denis: > > Add PG lock support as well as bank selection to > > the MMIO write function. > > > > Signed-off-by: Tom St Denis <tom.stdenis at amd.com > <mailto:tom.stdenis at amd.com>> > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 43 > ++++++++++++++++++++++++++++++ > > 1 file changed, 43 insertions(+) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > index 04c4aee70452..a9d83c1d7a43 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > @@ -2637,10 +2637,45 @@ static ssize_t > amdgpu_debugfs_regs_write(struct file *f, const char __user *buf, > > struct amdgpu_device *adev = f->f_inode->i_private; > > ssize_t result = 0; > > int r; > > + bool pm_pg_lock, use_bank; > > + unsigned instance_bank, sh_bank, se_bank; > > > > if (size & 0x3 || *pos & 0x3) > > return -EINVAL; > > > > + /* are we reading registers for which a PG lock is > necessary? */ > > + pm_pg_lock = (*pos >> 23) & 1; > > + > > + if (*pos & (1ULL << 62)) { > > + se_bank = (*pos >> 24) & 0x3FF; > > + sh_bank = (*pos >> 34) & 0x3FF; > > + instance_bank = (*pos >> 44) & 0x3FF; > > + > > + if (se_bank == 0x3FF) > > + se_bank = 0xFFFFFFFF; > > + if (sh_bank == 0x3FF) > > + sh_bank = 0xFFFFFFFF; > > + if (instance_bank == 0x3FF) > > + instance_bank = 0xFFFFFFFF; > > + use_bank = 1; > > + } else { > > + use_bank = 0; > > + } > > + > > + *pos &= 0x3FFFF; > > + > > + if (use_bank) { > > + if ((sh_bank != 0xFFFFFFFF && sh_bank >= > adev->gfx.config.max_sh_per_se) || > > + (se_bank != 0xFFFFFFFF && se_bank >= > adev->gfx.config.max_shader_engines)) > > + return -EINVAL; > > + mutex_lock(&adev->grbm_idx_mutex); > > + amdgpu_gfx_select_se_sh(adev, se_bank, > > + sh_bank, instance_bank); > > + } > > + > > + if (pm_pg_lock) > > + mutex_lock(&adev->pm.mutex); > > + > > while (size) { > > uint32_t value; > > > > @@ -2659,6 +2694,14 @@ static ssize_t > amdgpu_debugfs_regs_write(struct file *f, const char __user *buf, > > size -= 4; > > } > > > > + if (use_bank) { > > + amdgpu_gfx_select_se_sh(adev, 0xffffffff, > 0xffffffff, 0xffffffff); > > + mutex_unlock(&adev->grbm_idx_mutex); > > + } > > + > > + if (pm_pg_lock) > > + mutex_unlock(&adev->pm.mutex); > > + > > return result; > > } > > > > -------------- next part -------------- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20161010/f61a26f6/attachment.html>
