[PATCH] drm/amd/amdgpu: Make debugfs write compliment read

[Christian König]

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>