From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,UNWANTED_LANGUAGE_BODY, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 35D7DC4338F for ; Wed, 25 Aug 2021 11:03:27 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 00F38611C9 for ; Wed, 25 Aug 2021 11:03:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 00F38611C9 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id A5E9D6E1AA; Wed, 25 Aug 2021 11:03:26 +0000 (UTC) Received: from mail-wm1-x335.google.com (mail-wm1-x335.google.com [IPv6:2a00:1450:4864:20::335]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2F7356E1AA for ; Wed, 25 Aug 2021 11:03:25 +0000 (UTC) Received: by mail-wm1-x335.google.com with SMTP id u15so14699611wmj.1 for ; Wed, 25 Aug 2021 04:03:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language; bh=ovxb90O+5aZySWzamhusZsAJBODHt7zQ0+pYWTpOUu8=; b=OXqbKhQbhnWMfZpXzp97W3aknk3rIomzdGveXDh6JGntPJC3gvpE8YdMlSdZ+3eSLh xsSme1DP552hYEjBjxtBbexNk1DIei8rddQ1ltw64wSgV/0CGTg7cKIAKXXnuMqjEEJU WUl+0Rs87NCDg52skbsDiJlIoI5bKLMCwh+3rhnOAecTcRGIzvlSxkRrWD+nBSHr6lRl sMdigOoQhaqNqBf58tuUqxLABtTk6x2HLSaN96hbZu1XR8KJAjFF/O3ET121W9EdaNgv d3vIrMv1xtmoJ+DLK+cQmTpdHagoUBk9l4vWSl8rfBsWDhYMEVryzE8Idh/5/avE5rGN zvJg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language; bh=ovxb90O+5aZySWzamhusZsAJBODHt7zQ0+pYWTpOUu8=; b=Mo5FgeL/iNWkTEml/ELSlOR2xZsOJk963IMnwaF2KovzGoca/bjM4cHS1d8lp26Lzk SoIbhta+hR+IgMmor5d+TYhdukV1Xxh40C1Nqs28oEbLQfdHNwEDZTibArz9pmjh9nA7 S3+5apZ95O1wOtqVnMMjxYV1axIPDqhUhhKIDSS/qP7PrGfudDCU1pMI+ulZeIwWOD6p 2LDoaOuDPovc0HsY3m4NovC3lHsP5L3oI+xNChWEAEL00EgkypYdHFdzL/xx/l2wsvSG 0OYczhtnE9L9JZEmY5Mur2fpixeb02EcfTndnZCyFL6cr+zYIuhqecKyKyVYqy8JW0sG J4uA== X-Gm-Message-State: AOAM5307b9B6v/m1IsahakHNKoUdDr5R0ALUqHwHjXrdsl82h7ZZZ+Yy hNdNEXhTBv2l+OtZur+XaCmrBOcO8KJHLoUU X-Google-Smtp-Source: ABdhPJwIQknVFVVFm213KrtoKoZOe0ALnWd0LHd7h8gb7wSKQgE2K3C6DoGmo+jBQCP1XTI1g/zKTw== X-Received: by 2002:a05:600c:b51:: with SMTP id k17mr8759395wmr.149.1629889403603; Wed, 25 Aug 2021 04:03:23 -0700 (PDT) Received: from [192.168.178.21] (p5b0ea1b5.dip0.t-ipconnect.de. [91.14.161.181]) by smtp.gmail.com with ESMTPSA id f17sm5049571wmq.17.2021.08.25.04.03.22 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 25 Aug 2021 04:03:22 -0700 (PDT) Subject: Re: [PATCH] drm/amd/amdgpu: New debugfs interface for MMIO registers (v2) To: Tom St Denis , Nirmoy Das Cc: Tom St Denis , amd-gfx mailing list References: <20210824133642.109072-1-tom.stdenis@amd.com> From: =?UTF-8?Q?Christian_K=c3=b6nig?= Message-ID: <84ddce49-012b-2fae-d14d-eeebf7e6c09a@gmail.com> Date: Wed, 25 Aug 2021 13:03:21 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/alternative; boundary="------------8B85183ADFC3AF0999592E6A" Content-Language: en-US X-BeenThere: amd-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Discussion list for AMD gfx List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: amd-gfx-bounces@lists.freedesktop.org Sender: "amd-gfx" This is a multi-part message in MIME format. --------------8B85183ADFC3AF0999592E6A Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Using u8 is ok as well, just make sure that you don't have any hidden padding. Nirmoy had a tool to double check for paddings which I once more forgot the name of. Christian. Am 25.08.21 um 12:40 schrieb Tom St Denis: > The struct works as is but I'll change them to u32.  The offset is an > artefact of the fact this was an IOCTL originally.  I'm working both > ends in parallel trying to make the changes at the same time because > I'm only submitting the kernel patch if I've tested it in userspace. > > I'll send a v4 in a bit this morning.... > > Tom > > On Wed, Aug 25, 2021 at 2:35 AM Christian König > > wrote: > > > > Am 24.08.21 um 15:36 schrieb Tom St Denis: > > This new debugfs interface uses an IOCTL interface in order to pass > > along state information like SRBM and GRBM bank switching.  This > > new interface also allows a full 32-bit MMIO address range which > > the previous didn't.  With this new design we have room to grow > > the flexibility of the file as need be. > > > > (v2): Move read/write to .read/.write, fix style, add comment > >        for IOCTL data structure > > > > Signed-off-by: Tom St Denis > > > --- > >   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 162 > ++++++++++++++++++++ > >   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h |  32 ++++ > >   2 files changed, 194 insertions(+) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c > > index 277128846dd1..8e8f5743c8f5 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c > > @@ -279,6 +279,156 @@ static ssize_t > amdgpu_debugfs_regs_write(struct file *f, const char __user *buf, > >       return amdgpu_debugfs_process_reg_op(false, f, (char > __user *)buf, size, pos); > >   } > > > > +static int amdgpu_debugfs_regs2_open(struct inode *inode, > struct file *file) > > +{ > > +     struct amdgpu_debugfs_regs2_data *rd; > > + > > +     rd = kzalloc(sizeof *rd, GFP_KERNEL); > > +     if (!rd) > > +             return -ENOMEM; > > +     rd->adev = file_inode(file)->i_private; > > +     file->private_data = rd; > > + > > +     return 0; > > +} > > + > > +static int amdgpu_debugfs_regs2_release(struct inode *inode, > struct file *file) > > +{ > > +     kfree(file->private_data); > > +     return 0; > > +} > > + > > +static ssize_t amdgpu_debugfs_regs2_op(struct file *f, char > __user *buf, size_t size, int write_en) > > +{ > > +     struct amdgpu_debugfs_regs2_data *rd = f->private_data; > > +     struct amdgpu_device *adev = rd->adev; > > +     ssize_t result = 0; > > +     int r; > > +     uint32_t value; > > + > > +     if (size & 0x3 || rd->state.offset & 0x3) > > +             return -EINVAL; > > + > > +     if (rd->state.id.use_grbm) { > > +             if (rd->state.id.grbm.se > == 0x3FF) > > +                     rd->state.id.grbm.se > = 0xFFFFFFFF; > > +             if (rd->state.id.grbm.sh > == 0x3FF) > > +                     rd->state.id.grbm.sh > = 0xFFFFFFFF; > > +             if (rd->state.id.grbm.instance == 0x3FF) > > +                     rd->state.id.grbm.instance = 0xFFFFFFFF; > > +     } > > + > > +     r = pm_runtime_get_sync(adev_to_drm(adev)->dev); > > +     if (r < 0) { > > +  pm_runtime_put_autosuspend(adev_to_drm(adev)->dev); > > +             return r; > > +     } > > + > > +     r = amdgpu_virt_enable_access_debugfs(adev); > > +     if (r < 0) { > > +  pm_runtime_put_autosuspend(adev_to_drm(adev)->dev); > > +             return r; > > +     } > > + > > +     if (rd->state.id.use_grbm) { > > +             if ((rd->state.id.grbm.sh > != 0xFFFFFFFF && rd->state.id.grbm.sh > >= adev->gfx.config.max_sh_per_se) || > > +                 (rd->state.id.grbm.se > != 0xFFFFFFFF && rd->state.id.grbm.se > >= adev->gfx.config.max_shader_engines)) { > > +  pm_runtime_mark_last_busy(adev_to_drm(adev)->dev); > > +  pm_runtime_put_autosuspend(adev_to_drm(adev)->dev); > > +  amdgpu_virt_disable_access_debugfs(adev); > > +                     return -EINVAL; > > +             } > > +             mutex_lock(&adev->grbm_idx_mutex); > > +             amdgpu_gfx_select_se_sh(adev, rd->state.id.grbm.se > , > > +      rd->state.id.grbm.sh , > > +      rd->state.id.grbm.instance); > > +     } > > + > > +     if (rd->state.id.use_srbm) { > > +             mutex_lock(&adev->srbm_mutex); > > +             amdgpu_gfx_select_me_pipe_q(adev, > rd->state.id.srbm.me , > rd->state.id.srbm.pipe, > > +              rd->state.id.srbm.queue, rd->state.id.srbm.vmid); > > +     } > > + > > +     if (rd->state.id.pg_lock) > > +             mutex_lock(&adev->pm.mutex); > > + > > +     while (size) { > > +             if (!write_en) { > > +                     value = RREG32(rd->state.offset >> 2); > > +                     r = put_user(value, (uint32_t *)buf); > > +             } else { > > +                     r = get_user(value, (uint32_t *)buf); > > +                     if (!r) > > +  amdgpu_mm_wreg_mmio_rlc(adev, rd->state.offset >> 2, value); > > +             } > > +             if (r) { > > +                     result = r; > > +                     goto end; > > +             } > > +             rd->state.offset += 4; > > +             size -= 4; > > +             result += 4; > > +             buf += 4; > > +     } > > +end: > > +     if (rd->state.id.use_grbm) { > > +             amdgpu_gfx_select_se_sh(adev, 0xffffffff, > 0xffffffff, 0xffffffff); > > +             mutex_unlock(&adev->grbm_idx_mutex); > > +     } > > + > > +     if (rd->state.id.use_srbm) { > > +             amdgpu_gfx_select_me_pipe_q(adev, 0, 0, 0, 0); > > +             mutex_unlock(&adev->srbm_mutex); > > +     } > > + > > +     if (rd->state.id.pg_lock) > > +             mutex_unlock(&adev->pm.mutex); > > + > > +     // in umr (the likely user of this) flags are set per file > operation > > +     // which means they're never "unset" explicitly. To avoid > breaking > > +     // this convention we unset the flags after each operation > > +     // flags are for a single call (need to be set for every > read/write) > > +     rd->state.id.use_grbm = 0; > > +     rd->state.id.use_srbm = 0; > > +     rd->state.id.pg_lock  = 0; > > + > > +  pm_runtime_mark_last_busy(adev_to_drm(adev)->dev); > > +  pm_runtime_put_autosuspend(adev_to_drm(adev)->dev); > > + > > +     amdgpu_virt_disable_access_debugfs(adev); > > +     return result; > > +} > > + > > +static long amdgpu_debugfs_regs2_ioctl(struct file *f, unsigned > int cmd, unsigned long data) > > +{ > > +     struct amdgpu_debugfs_regs2_data *rd = f->private_data; > > + > > +     switch (cmd) { > > +     case AMDGPU_DEBUGFS_REGS2_IOC_SET_STATE: > > +             if (copy_from_user(&rd->state.id > , (struct amdgpu_debugfs_regs2_iocdata *)data, > sizeof rd->state.id )) > > +                     return -EINVAL; > > +             break; > > +     default: > > +             return -EINVAL; > > +     } > > +     return 0; > > +} > > + > > +static ssize_t amdgpu_debugfs_regs2_read(struct file *f, char > __user *buf, size_t size, loff_t *pos) > > +{ > > +     struct amdgpu_debugfs_regs2_data *rd = f->private_data; > > +     rd->state.offset = *pos; > > +     return amdgpu_debugfs_regs2_op(f, buf, size, 0); > > +} > > + > > +static ssize_t amdgpu_debugfs_regs2_write(struct file *f, const > char __user *buf, size_t size, loff_t *pos) > > +{ > > +     struct amdgpu_debugfs_regs2_data *rd = f->private_data; > > +     rd->state.offset = *pos; > > +     return amdgpu_debugfs_regs2_op(f, (char __user *)buf, > size, 1); > > +} > > + > > > >   /** > >    * amdgpu_debugfs_regs_pcie_read - Read from a PCIE register > > @@ -1091,6 +1241,16 @@ static ssize_t > amdgpu_debugfs_gfxoff_read(struct file *f, char __user *buf, > >       return result; > >   } > > > > +static const struct file_operations amdgpu_debugfs_regs2_fops = { > > +     .owner = THIS_MODULE, > > +     .unlocked_ioctl = amdgpu_debugfs_regs2_ioctl, > > +     .read = amdgpu_debugfs_regs2_read, > > +     .write = amdgpu_debugfs_regs2_write, > > +     .open = amdgpu_debugfs_regs2_open, > > +     .release = amdgpu_debugfs_regs2_release, > > +     .llseek = default_llseek > > +}; > > + > >   static const struct file_operations amdgpu_debugfs_regs_fops = { > >       .owner = THIS_MODULE, > >       .read = amdgpu_debugfs_regs_read, > > @@ -1148,6 +1308,7 @@ static const struct file_operations > amdgpu_debugfs_gfxoff_fops = { > > > >   static const struct file_operations *debugfs_regs[] = { > >       &amdgpu_debugfs_regs_fops, > > +     &amdgpu_debugfs_regs2_fops, > >       &amdgpu_debugfs_regs_didt_fops, > >       &amdgpu_debugfs_regs_pcie_fops, > >       &amdgpu_debugfs_regs_smc_fops, > > @@ -1160,6 +1321,7 @@ static const struct file_operations > *debugfs_regs[] = { > > > >   static const char *debugfs_regs_names[] = { > >       "amdgpu_regs", > > +     "amdgpu_regs2", > >       "amdgpu_regs_didt", > >       "amdgpu_regs_pcie", > >       "amdgpu_regs_smc", > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h > > index 141a8474e24f..ec044df5d428 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h > > @@ -22,6 +22,8 @@ > >    * OTHER DEALINGS IN THE SOFTWARE. > >    * > >    */ > > +#include > > +#include > > > >   /* > >    * Debugfs > > @@ -38,3 +40,33 @@ void amdgpu_debugfs_fence_init(struct > amdgpu_device *adev); > >   void amdgpu_debugfs_firmware_init(struct amdgpu_device *adev); > >   void amdgpu_debugfs_gem_init(struct amdgpu_device *adev); > >   int amdgpu_debugfs_wait_dump(struct amdgpu_device *adev); > > + > > +/* > > + * MMIO debugfs IOCTL structure > > + */ > > +struct amdgpu_debugfs_regs2_iocdata { > > +     __u8 use_srbm, use_grbm, pg_lock; > > You should consider using u32 here as well or add explicitly padding. > > > +     struct { > > +             __u32 se, sh, instance; > > +     } grbm; > > +     struct { > > +             __u32 me, pipe, queue, vmid; > > +     } srbm; > > +}; > > + > > +/* > > + * MMIO debugfs state data (per file* handle) > > + */ > > +struct amdgpu_debugfs_regs2_data { > > +     struct amdgpu_device *adev; > > +     struct { > > +             struct amdgpu_debugfs_regs2_iocdata id; > > +             __u32 offset; > > What is the offset good for here? > > Regards, > Christian. > > > +     } state; > > +}; > > + > > +enum AMDGPU_DEBUGFS_REGS2_CMDS { > > +     AMDGPU_DEBUGFS_REGS2_CMD_SET_STATE=0, > > +}; > > + > > +#define AMDGPU_DEBUGFS_REGS2_IOC_SET_STATE _IOWR(0x20, > AMDGPU_DEBUGFS_REGS2_CMD_SET_STATE, struct > amdgpu_debugfs_regs2_iocdata) > --------------8B85183ADFC3AF0999592E6A Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 8bit Using u8 is ok as well, just make sure that you don't have any hidden padding.

Nirmoy had a tool to double check for paddings which I once more forgot the name of.

Christian.

Am 25.08.21 um 12:40 schrieb Tom St Denis:
The struct works as is but I'll change them to u32.  The offset is an artefact of the fact this was an IOCTL originally.  I'm working both ends in parallel trying to make the changes at the same time because I'm only submitting the kernel patch if I've tested it in userspace.

I'll send a v4 in a bit this morning....

Tom

On Wed, Aug 25, 2021 at 2:35 AM Christian König <ckoenig.leichtzumerken@gmail.com> wrote:


Am 24.08.21 um 15:36 schrieb Tom St Denis:
> This new debugfs interface uses an IOCTL interface in order to pass
> along state information like SRBM and GRBM bank switching.  This
> new interface also allows a full 32-bit MMIO address range which
> the previous didn't.  With this new design we have room to grow
> the flexibility of the file as need be.
>
> (v2): Move read/write to .read/.write, fix style, add comment
>        for IOCTL data structure
>
> Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 162 ++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h |  32 ++++
>   2 files changed, 194 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> index 277128846dd1..8e8f5743c8f5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> @@ -279,6 +279,156 @@ static ssize_t amdgpu_debugfs_regs_write(struct file *f, const char __user *buf,
>       return amdgpu_debugfs_process_reg_op(false, f, (char __user *)buf, size, pos);
>   }
>   
> +static int amdgpu_debugfs_regs2_open(struct inode *inode, struct file *file)
> +{
> +     struct amdgpu_debugfs_regs2_data *rd;
> +
> +     rd = kzalloc(sizeof *rd, GFP_KERNEL);
> +     if (!rd)
> +             return -ENOMEM;
> +     rd->adev = file_inode(file)->i_private;
> +     file->private_data = rd;
> +
> +     return 0;
> +}
> +
> +static int amdgpu_debugfs_regs2_release(struct inode *inode, struct file *file)
> +{
> +     kfree(file->private_data);
> +     return 0;
> +}
> +
> +static ssize_t amdgpu_debugfs_regs2_op(struct file *f, char __user *buf, size_t size, int write_en)
> +{
> +     struct amdgpu_debugfs_regs2_data *rd = f->private_data;
> +     struct amdgpu_device *adev = rd->adev;
> +     ssize_t result = 0;
> +     int r;
> +     uint32_t value;
> +
> +     if (size & 0x3 || rd->state.offset & 0x3)
> +             return -EINVAL;
> +
> +     if (rd->state.id.use_grbm) {
> +             if (rd->state.id.grbm.se == 0x3FF)
> +                     rd->state.id.grbm.se = 0xFFFFFFFF;
> +             if (rd->state.id.grbm.sh == 0x3FF)
> +                     rd->state.id.grbm.sh = 0xFFFFFFFF;
> +             if (rd->state.id.grbm.instance == 0x3FF)
> +                     rd->state.id.grbm.instance = 0xFFFFFFFF;
> +     }
> +
> +     r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
> +     if (r < 0) {
> +             pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> +             return r;
> +     }
> +
> +     r = amdgpu_virt_enable_access_debugfs(adev);
> +     if (r < 0) {
> +             pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> +             return r;
> +     }
> +
> +     if (rd->state.id.use_grbm) {
> +             if ((rd->state.id.grbm.sh != 0xFFFFFFFF && rd->state.id.grbm.sh >= adev->gfx.config.max_sh_per_se) ||
> +                 (rd->state.id.grbm.se != 0xFFFFFFFF && rd->state.id.grbm.se >= adev->gfx.config.max_shader_engines)) {
> +                     pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> +                     pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> +                     amdgpu_virt_disable_access_debugfs(adev);
> +                     return -EINVAL;
> +             }
> +             mutex_lock(&adev->grbm_idx_mutex);
> +             amdgpu_gfx_select_se_sh(adev, rd->state.id.grbm.se,
> +                                                             rd->state.id.grbm.sh,
> +                                                             rd->state.id.grbm.instance);
> +     }
> +
> +     if (rd->state.id.use_srbm) {
> +             mutex_lock(&adev->srbm_mutex);
> +             amdgpu_gfx_select_me_pipe_q(adev, rd->state.id.srbm.me, rd->state.id.srbm.pipe,
> +                                                                     rd->state.id.srbm.queue, rd->state.id.srbm.vmid);
> +     }
> +
> +     if (rd->state.id.pg_lock)
> +             mutex_lock(&adev->pm.mutex);
> +
> +     while (size) {
> +             if (!write_en) {
> +                     value = RREG32(rd->state.offset >> 2);
> +                     r = put_user(value, (uint32_t *)buf);
> +             } else {
> +                     r = get_user(value, (uint32_t *)buf);
> +                     if (!r)
> +                             amdgpu_mm_wreg_mmio_rlc(adev, rd->state.offset >> 2, value);
> +             }
> +             if (r) {
> +                     result = r;
> +                     goto end;
> +             }
> +             rd->state.offset += 4;
> +             size -= 4;
> +             result += 4;
> +             buf += 4;
> +     }
> +end:
> +     if (rd->state.id.use_grbm) {
> +             amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff);
> +             mutex_unlock(&adev->grbm_idx_mutex);
> +     }
> +
> +     if (rd->state.id.use_srbm) {
> +             amdgpu_gfx_select_me_pipe_q(adev, 0, 0, 0, 0);
> +             mutex_unlock(&adev->srbm_mutex);
> +     }
> +
> +     if (rd->state.id.pg_lock)
> +             mutex_unlock(&adev->pm.mutex);
> +
> +     // in umr (the likely user of this) flags are set per file operation
> +     // which means they're never "unset" explicitly.  To avoid breaking
> +     // this convention we unset the flags after each operation
> +     // flags are for a single call (need to be set for every read/write)
> +     rd->state.id.use_grbm = 0;
> +     rd->state.id.use_srbm = 0;
> +     rd->state.id.pg_lock  = 0;
> +
> +     pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> +     pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> +
> +     amdgpu_virt_disable_access_debugfs(adev);
> +     return result;
> +}
> +
> +static long amdgpu_debugfs_regs2_ioctl(struct file *f, unsigned int cmd, unsigned long data)
> +{
> +     struct amdgpu_debugfs_regs2_data *rd = f->private_data;
> +
> +     switch (cmd) {
> +     case AMDGPU_DEBUGFS_REGS2_IOC_SET_STATE:
> +             if (copy_from_user(&rd->state.id, (struct amdgpu_debugfs_regs2_iocdata *)data, sizeof rd->state.id))
> +                     return -EINVAL;
> +             break;
> +     default:
> +             return -EINVAL;
> +     }
> +     return 0;
> +}
> +
> +static ssize_t amdgpu_debugfs_regs2_read(struct file *f, char __user *buf, size_t size, loff_t *pos)
> +{
> +     struct amdgpu_debugfs_regs2_data *rd = f->private_data;
> +     rd->state.offset = *pos;
> +     return amdgpu_debugfs_regs2_op(f, buf, size, 0);
> +}
> +
> +static ssize_t amdgpu_debugfs_regs2_write(struct file *f, const char __user *buf, size_t size, loff_t *pos)
> +{
> +     struct amdgpu_debugfs_regs2_data *rd = f->private_data;
> +     rd->state.offset = *pos;
> +     return amdgpu_debugfs_regs2_op(f, (char __user *)buf, size, 1);
> +}
> +
>   
>   /**
>    * amdgpu_debugfs_regs_pcie_read - Read from a PCIE register
> @@ -1091,6 +1241,16 @@ static ssize_t amdgpu_debugfs_gfxoff_read(struct file *f, char __user *buf,
>       return result;
>   }
>   
> +static const struct file_operations amdgpu_debugfs_regs2_fops = {
> +     .owner = THIS_MODULE,
> +     .unlocked_ioctl = amdgpu_debugfs_regs2_ioctl,
> +     .read = amdgpu_debugfs_regs2_read,
> +     .write = amdgpu_debugfs_regs2_write,
> +     .open = amdgpu_debugfs_regs2_open,
> +     .release = amdgpu_debugfs_regs2_release,
> +     .llseek = default_llseek
> +};
> +
>   static const struct file_operations amdgpu_debugfs_regs_fops = {
>       .owner = THIS_MODULE,
>       .read = amdgpu_debugfs_regs_read,
> @@ -1148,6 +1308,7 @@ static const struct file_operations amdgpu_debugfs_gfxoff_fops = {
>   
>   static const struct file_operations *debugfs_regs[] = {
>       &amdgpu_debugfs_regs_fops,
> +     &amdgpu_debugfs_regs2_fops,
>       &amdgpu_debugfs_regs_didt_fops,
>       &amdgpu_debugfs_regs_pcie_fops,
>       &amdgpu_debugfs_regs_smc_fops,
> @@ -1160,6 +1321,7 @@ static const struct file_operations *debugfs_regs[] = {
>   
>   static const char *debugfs_regs_names[] = {
>       "amdgpu_regs",
> +     "amdgpu_regs2",
>       "amdgpu_regs_didt",
>       "amdgpu_regs_pcie",
>       "amdgpu_regs_smc",
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h
> index 141a8474e24f..ec044df5d428 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h
> @@ -22,6 +22,8 @@
>    * OTHER DEALINGS IN THE SOFTWARE.
>    *
>    */
> +#include <linux/ioctl.h>
> +#include <uapi/drm/amdgpu_drm.h>
>   
>   /*
>    * Debugfs
> @@ -38,3 +40,33 @@ void amdgpu_debugfs_fence_init(struct amdgpu_device *adev);
>   void amdgpu_debugfs_firmware_init(struct amdgpu_device *adev);
>   void amdgpu_debugfs_gem_init(struct amdgpu_device *adev);
>   int amdgpu_debugfs_wait_dump(struct amdgpu_device *adev);
> +
> +/*
> + * MMIO debugfs IOCTL structure
> + */
> +struct amdgpu_debugfs_regs2_iocdata {
> +     __u8 use_srbm, use_grbm, pg_lock;

You should consider using u32 here as well or add explicitly padding.

> +     struct {
> +             __u32 se, sh, instance;
> +     } grbm;
> +     struct {
> +             __u32 me, pipe, queue, vmid;
> +     } srbm;
> +};
> +
> +/*
> + * MMIO debugfs state data (per file* handle)
> + */
> +struct amdgpu_debugfs_regs2_data {
> +     struct amdgpu_device *adev;
> +     struct {
> +             struct amdgpu_debugfs_regs2_iocdata id;
> +             __u32 offset;

What is the offset good for here?

Regards,
Christian.

> +     } state;
> +};
> +
> +enum AMDGPU_DEBUGFS_REGS2_CMDS {
> +     AMDGPU_DEBUGFS_REGS2_CMD_SET_STATE=0,
> +};
> +
> +#define AMDGPU_DEBUGFS_REGS2_IOC_SET_STATE _IOWR(0x20, AMDGPU_DEBUGFS_REGS2_CMD_SET_STATE, struct amdgpu_debugfs_regs2_iocdata)


--------------8B85183ADFC3AF0999592E6A--