From: "Christian König" <ckoenig.leichtzumerken@gmail.com>
To: "Hawking Zhang" <Hawking.Zhang@amd.com>,
amd-gfx@lists.freedesktop.org,
"Alex Deucher" <alexander.deucher@amd.com>,
"Kevin Wang" <kevin1.wang@amd.com>,
"Christian König" <christian.koenig@amd.com>,
"Guchun Chen" <guchun.chen@amd.com>
Subject: Re: [PATCH 3/3] drm/amdgpu: support indirect access reg outside of mmio bar (v2)
Date: Fri, 18 Sep 2020 14:51:20 +0200 [thread overview]
Message-ID: <ee9791e0-4df4-abd9-fb85-17d7e2424b2a@gmail.com> (raw)
In-Reply-To: <20200918123747.7843-3-Hawking.Zhang@amd.com>
Am 18.09.20 um 14:37 schrieb Hawking Zhang:
> support both direct and indirect accessor in unified
> helper functions.
>
> v2: Retire indirect mmio access via mm_index/data
>
> Signed-off-by: Hawking Zhang <Hawking.Zhang@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 23 +++----
> drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 95 ++++++++++++-----------------
> drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 4 +-
> 4 files changed, 53 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 40ee44b..50341f0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1019,12 +1019,13 @@ int amdgpu_gpu_wait_for_idle(struct amdgpu_device *adev);
>
> void amdgpu_device_vram_access(struct amdgpu_device *adev, loff_t pos,
> uint32_t *buf, size_t size, bool write);
> -uint32_t amdgpu_mm_rreg(struct amdgpu_device *adev, uint32_t reg,
> +uint32_t amdgpu_device_rreg(struct amdgpu_device *adev,
> + uint32_t reg, uint32_t acc_flags);
> +void amdgpu_device_wreg(struct amdgpu_device *adev,
> + uint32_t reg, uint32_t v,
> uint32_t acc_flags);
> -void amdgpu_mm_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v,
> - uint32_t acc_flags);
> -void amdgpu_mm_wreg_mmio_rlc(struct amdgpu_device *adev, uint32_t reg, uint32_t v,
> - uint32_t acc_flags);
> +void amdgpu_mm_wreg_mmio_rlc(struct amdgpu_device *adev,
> + uint32_t reg, uint32_t v);
> void amdgpu_mm_wreg8(struct amdgpu_device *adev, uint32_t offset, uint8_t value);
> uint8_t amdgpu_mm_rreg8(struct amdgpu_device *adev, uint32_t offset);
>
> @@ -1054,8 +1055,8 @@ int emu_soc_asic_init(struct amdgpu_device *adev);
> */
> #define AMDGPU_REGS_NO_KIQ (1<<1)
>
> -#define RREG32_NO_KIQ(reg) amdgpu_mm_rreg(adev, (reg), AMDGPU_REGS_NO_KIQ)
> -#define WREG32_NO_KIQ(reg, v) amdgpu_mm_wreg(adev, (reg), (v), AMDGPU_REGS_NO_KIQ)
> +#define RREG32_NO_KIQ(reg) amdgpu_device_rreg(adev, (reg), AMDGPU_REGS_NO_KIQ)
> +#define WREG32_NO_KIQ(reg, v) amdgpu_device_wreg(adev, (reg), (v), AMDGPU_REGS_NO_KIQ)
>
> #define RREG32_KIQ(reg) amdgpu_kiq_rreg(adev, (reg))
> #define WREG32_KIQ(reg, v) amdgpu_kiq_wreg(adev, (reg), (v))
> @@ -1063,9 +1064,9 @@ int emu_soc_asic_init(struct amdgpu_device *adev);
> #define RREG8(reg) amdgpu_mm_rreg8(adev, (reg))
> #define WREG8(reg, v) amdgpu_mm_wreg8(adev, (reg), (v))
>
> -#define RREG32(reg) amdgpu_mm_rreg(adev, (reg), 0)
> -#define DREG32(reg) printk(KERN_INFO "REGISTER: " #reg " : 0x%08X\n", amdgpu_mm_rreg(adev, (reg), 0))
> -#define WREG32(reg, v) amdgpu_mm_wreg(adev, (reg), (v), 0)
> +#define RREG32(reg) amdgpu_device_rreg(adev, (reg), 0)
> +#define DREG32(reg) printk(KERN_INFO "REGISTER: " #reg " : 0x%08X\n", amdgpu_device_rreg(adev, (reg), 0))
> +#define WREG32(reg, v) amdgpu_device_wreg(adev, (reg), (v), 0)
> #define REG_SET(FIELD, v) (((v) << FIELD##_SHIFT) & FIELD##_MASK)
> #define REG_GET(FIELD, v) (((v) << FIELD##_SHIFT) & FIELD##_MASK)
> #define RREG32_PCIE(reg) adev->pcie_rreg(adev, (reg))
> @@ -1111,7 +1112,7 @@ int emu_soc_asic_init(struct amdgpu_device *adev);
> WREG32_SMC(_Reg, tmp); \
> } while (0)
>
> -#define DREG32_SYS(sqf, adev, reg) seq_printf((sqf), #reg " : 0x%08X\n", amdgpu_mm_rreg((adev), (reg), false))
> +#define DREG32_SYS(sqf, adev, reg) seq_printf((sqf), #reg " : 0x%08X\n", amdgpu_device_rreg((adev), (reg), false))
> #define RREG32_IO(reg) amdgpu_io_rreg(adev, (reg))
> #define WREG32_IO(reg, v) amdgpu_io_wreg(adev, (reg), (v))
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> index abe0c27..2d125b8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> @@ -267,7 +267,7 @@ static int amdgpu_debugfs_process_reg_op(bool read, struct file *f,
> } else {
> r = get_user(value, (uint32_t *)buf);
> if (!r)
> - amdgpu_mm_wreg_mmio_rlc(adev, *pos >> 2, value, 0);
> + amdgpu_mm_wreg_mmio_rlc(adev, *pos >> 2, value);
> }
> if (r) {
> result = r;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 77785b2..365ced6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -303,10 +303,10 @@ void amdgpu_device_vram_access(struct amdgpu_device *adev, loff_t pos,
> }
>
> /*
> - * MMIO register access helper functions.
> + * register access helper functions.
> */
> /**
> - * amdgpu_mm_rreg - read a memory mapped IO register
> + * amdgpu_device_rreg - read a memory mapped IO or indirect register
> *
> * @adev: amdgpu_device pointer
> * @reg: dword aligned register offset
> @@ -314,33 +314,29 @@ void amdgpu_device_vram_access(struct amdgpu_device *adev, loff_t pos,
> *
> * Returns the 32 bit value from the offset specified.
> */
> -uint32_t amdgpu_mm_rreg(struct amdgpu_device *adev, uint32_t reg,
> - uint32_t acc_flags)
> +uint32_t amdgpu_device_rreg(struct amdgpu_device *adev,
> + uint32_t reg, uint32_t acc_flags)
> {
> uint32_t ret;
>
> if (adev->in_pci_err_recovery)
> return 0;
>
> - if (!(acc_flags & AMDGPU_REGS_NO_KIQ) && amdgpu_sriov_runtime(adev) &&
> - down_read_trylock(&adev->reset_sem)) {
> - ret = amdgpu_kiq_rreg(adev, reg);
> - up_read(&adev->reset_sem);
> - return ret;
> + if ((reg * 4) < adev->rmmio_size) {
> + if (!(acc_flags & AMDGPU_REGS_NO_KIQ) &&
> + amdgpu_sriov_runtime(adev) &&
> + down_read_trylock(&adev->reset_sem)) {
> + ret = amdgpu_kiq_rreg(adev, reg);
> + up_read(&adev->reset_sem);
> + } else {
> + ret = readl(((void __iomem *)adev->rmmio) + (reg * 4));
> + }
> + } else {
> + ret = adev->pcie_rreg(adev, reg * 4);
> }
>
> - if ((reg * 4) < adev->rmmio_size)
> - ret = readl(((void __iomem *)adev->rmmio) + (reg * 4));
> - else {
> - unsigned long flags;
> -
> - spin_lock_irqsave(&adev->mmio_idx_lock, flags);
> - writel((reg * 4), ((void __iomem *)adev->rmmio) + (mmMM_INDEX * 4));
> - ret = readl(((void __iomem *)adev->rmmio) + (mmMM_DATA * 4));
> - spin_unlock_irqrestore(&adev->mmio_idx_lock, flags);
> - }
> + trace_amdgpu_device_rreg(adev->pdev->device, reg, ret);
>
> - trace_amdgpu_mm_rreg(adev->pdev->device, reg, ret);
> return ret;
> }
>
> @@ -394,29 +390,8 @@ void amdgpu_mm_wreg8(struct amdgpu_device *adev, uint32_t offset, uint8_t value)
> BUG();
> }
>
> -static inline void amdgpu_mm_wreg_mmio(struct amdgpu_device *adev,
> - uint32_t reg, uint32_t v,
> - uint32_t acc_flags)
> -{
> - if (adev->in_pci_err_recovery)
> - return;
> -
> - trace_amdgpu_mm_wreg(adev->pdev->device, reg, v);
> -
> - if ((reg * 4) < adev->rmmio_size)
> - writel(v, ((void __iomem *)adev->rmmio) + (reg * 4));
> - else {
> - unsigned long flags;
> -
> - spin_lock_irqsave(&adev->mmio_idx_lock, flags);
> - writel((reg * 4), ((void __iomem *)adev->rmmio) + (mmMM_INDEX * 4));
> - writel(v, ((void __iomem *)adev->rmmio) + (mmMM_DATA * 4));
> - spin_unlock_irqrestore(&adev->mmio_idx_lock, flags);
> - }
> -}
> -
> /**
> - * amdgpu_mm_wreg - write to a memory mapped IO register
> + * amdgpu_device_wreg - write to a memory mapped IO or indirect register
> *
> * @adev: amdgpu_device pointer
> * @reg: dword aligned register offset
> @@ -425,20 +400,27 @@ static inline void amdgpu_mm_wreg_mmio(struct amdgpu_device *adev,
> *
> * Writes the value specified to the offset specified.
> */
> -void amdgpu_mm_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v,
> - uint32_t acc_flags)
> +void amdgpu_device_wreg(struct amdgpu_device *adev,
> + uint32_t reg, uint32_t v,
> + uint32_t acc_flags)
> {
> if (adev->in_pci_err_recovery)
> return;
>
> - if (!(acc_flags & AMDGPU_REGS_NO_KIQ) && amdgpu_sriov_runtime(adev) &&
> - down_read_trylock(&adev->reset_sem)) {
> - amdgpu_kiq_wreg(adev, reg, v);
> - up_read(&adev->reset_sem);
> - return;
> + if ((reg * 4) < adev->rmmio_size) {
> + if (!(acc_flags & AMDGPU_REGS_NO_KIQ) &&
> + amdgpu_sriov_runtime(adev) &&
> + down_read_trylock(&adev->reset_sem)) {
> + amdgpu_kiq_wreg(adev, reg, v);
> + up_read(&adev->reset_sem);
> + } else {
> + writel(v, ((void __iomem *)adev->rmmio) + (reg * 4));
> + }
> + } else {
> + adev->pcie_wreg(adev, reg * 4, v);
> }
>
> - amdgpu_mm_wreg_mmio(adev, reg, v, acc_flags);
> + trace_amdgpu_device_wreg(adev->pdev->device, reg, v);
> }
>
> /*
> @@ -446,21 +428,20 @@ void amdgpu_mm_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v,
> *
> * this function is invoked only the debugfs register access
> * */
> -void amdgpu_mm_wreg_mmio_rlc(struct amdgpu_device *adev, uint32_t reg, uint32_t v,
> - uint32_t acc_flags)
> +void amdgpu_mm_wreg_mmio_rlc(struct amdgpu_device *adev,
> + uint32_t reg, uint32_t v)
> {
> if (adev->in_pci_err_recovery)
> return;
>
> if (amdgpu_sriov_fullaccess(adev) &&
> - adev->gfx.rlc.funcs &&
> - adev->gfx.rlc.funcs->is_rlcg_access_range) {
> -
> + adev->gfx.rlc.funcs &&
> + adev->gfx.rlc.funcs->is_rlcg_access_range) {
> if (adev->gfx.rlc.funcs->is_rlcg_access_range(adev, reg))
> return adev->gfx.rlc.funcs->rlcg_wreg(adev, reg, v);
> + } else {
> + writel(v, ((void __iomem *)adev->rmmio) + (reg * 4));
> }
> -
> - amdgpu_mm_wreg_mmio(adev, reg, v, acc_flags);
> }
>
> /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> index 63e734a..5da20fc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> @@ -35,7 +35,7 @@
> #define AMDGPU_JOB_GET_TIMELINE_NAME(job) \
> job->base.s_fence->finished.ops->get_timeline_name(&job->base.s_fence->finished)
>
> -TRACE_EVENT(amdgpu_mm_rreg,
> +TRACE_EVENT(amdgpu_device_rreg,
> TP_PROTO(unsigned did, uint32_t reg, uint32_t value),
> TP_ARGS(did, reg, value),
> TP_STRUCT__entry(
> @@ -54,7 +54,7 @@ TRACE_EVENT(amdgpu_mm_rreg,
> (unsigned long)__entry->value)
> );
>
> -TRACE_EVENT(amdgpu_mm_wreg,
> +TRACE_EVENT(amdgpu_device_wreg,
> TP_PROTO(unsigned did, uint32_t reg, uint32_t value),
> TP_ARGS(did, reg, value),
> TP_STRUCT__entry(
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
next prev parent reply other threads:[~2020-09-18 12:51 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-18 12:37 [PATCH 1/3] drm/amdgpu: add helper function for indirect reg access (v3) Hawking Zhang
2020-09-18 12:37 ` [PATCH 2/3] drm/amdgpu: switch to indirect reg access helper Hawking Zhang
2020-09-18 12:37 ` [PATCH 3/3] drm/amdgpu: support indirect access reg outside of mmio bar (v2) Hawking Zhang
2020-09-18 12:51 ` Christian König [this message]
-- strict thread matches above, loose matches on Subject: below --
2020-09-18 9:51 [PATCH 1/3] drm/amdgpu: add helper function for indirect reg access (v3) Hawking Zhang
2020-09-18 9:51 ` [PATCH 3/3] drm/amdgpu: support indirect access reg outside of mmio bar (v2) Hawking Zhang
2020-09-18 11:27 ` Christian König
2020-09-18 12:06 ` Zhang, Hawking
2020-09-18 12:12 ` Russell, Kent
2020-09-18 12:17 ` Christian König
2020-09-18 13:32 ` Russell, Kent
2020-09-18 13:51 ` Christian König
2020-09-18 14:02 ` Zhang, Hawking
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ee9791e0-4df4-abd9-fb85-17d7e2424b2a@gmail.com \
--to=ckoenig.leichtzumerken@gmail.com \
--cc=Hawking.Zhang@amd.com \
--cc=alexander.deucher@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=christian.koenig@amd.com \
--cc=guchun.chen@amd.com \
--cc=kevin1.wang@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.