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=-8.9 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,HTML_FONT_LOW_CONTRAST,HTML_MESSAGE, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,MIME_HTML_MOSTLY, NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,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 A6F12C2B9F4 for ; Mon, 28 Jun 2021 15:34:42 +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 63AC661465 for ; Mon, 28 Jun 2021 15:34:42 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 63AC661465 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=amd-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1F3BE899A5; Mon, 28 Jun 2021 15:34:42 +0000 (UTC) Received: from mail-ed1-x52d.google.com (mail-ed1-x52d.google.com [IPv6:2a00:1450:4864:20::52d]) by gabe.freedesktop.org (Postfix) with ESMTPS id 45EE2899A5 for ; Mon, 28 Jun 2021 15:34:40 +0000 (UTC) Received: by mail-ed1-x52d.google.com with SMTP id h2so26589762edt.3 for ; Mon, 28 Jun 2021 08:34:40 -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=bnbWD+J8SplZhuiPSpqxbbkLSwoS8/TISbOPmTgw47E=; b=b7btceCt4JqUgE++HUi5klaNeeAC5x+PYPi4gf3n8MRIck/Vy5ZIbXlL6UcaguFN6y aketovORC/BBC5IW+ZD3r67O5LKfkJ/qdg0mZTnTCfGQCfIfmScaYAX8//1hCtlYFrAb 3IKpjd4ZRO8mEme99eHM92H1YaaBF8ixAyv87GTB4ZriOy0PUh1hpFMM9YJtxnofj4bT WOnVFULbKMfUZZfhhn0ZHprPZqzkbaRbu0WNtixhnj3i8JuX8c/yJQIWXE6GW6wLNacX ToqN56zbKJ4TxuWXN6lOD3eTCQPTuvAzeaTp9oADCYoiHlL+MSFk3W6yUpKos4Bc5f2D Dggw== 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=bnbWD+J8SplZhuiPSpqxbbkLSwoS8/TISbOPmTgw47E=; b=InGOURZYW28Fz5cPd3a85L8v0W2ZMiG4o9+VHi0ig80xK3S5TO0HHphcBpMFS5vUIE x/B/Oad52UtxhZZXSkoJBA3s+eCInhN3WJbIUsLpAbGrkSWjbTQPMzS6jZu4vQLWTS4X uRSzAT7IqutexRcccJMdJCn4D1/OwcUdUxMHHviCnz8iJl+wKr3iGA9xB3fkouxDZK2x bylRjZQBRVorThVX3WI6OEKjNDo6AgAWcUBJmDUYo3JPWCv+pwItFFsy/cJfSLYjZSDF wMtZK7y2zJh6moqO8i+CAic/sYOH154LV+GAeuW0UgfqgSUjSlGv3PTupxdgRqsb9ucG Ra+w== X-Gm-Message-State: AOAM531kNPTIwan2HfxPPXY7qzgYB/IxoASIgnzSyFSXyf6Bd2bMKMj/ PVr2rpuD3/nxw4yTm3uk5qHNk2RbxKc= X-Google-Smtp-Source: ABdhPJyDMg7NkPoAS/iRRx/p6kUcc5IEyqSsSQMDn8wZ8tz/EOimgni+cLbWgmznEWhwbEd4Be9DUw== X-Received: by 2002:a05:6402:10c3:: with SMTP id p3mr31253783edu.175.1624894478935; Mon, 28 Jun 2021 08:34:38 -0700 (PDT) Received: from ?IPv6:2a02:908:1252:fb60:4650:114e:394c:f6d4? ([2a02:908:1252:fb60:4650:114e:394c:f6d4]) by smtp.gmail.com with ESMTPSA id l26sm9958039edt.40.2021.06.28.08.34.37 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 28 Jun 2021 08:34:38 -0700 (PDT) Subject: Re: [PATCH] drm/amdgpu: add non-aligned address supported in amdgpu_device_vram_access() To: "Wang, Kevin(Yang)" , "amd-gfx@lists.freedesktop.org" References: <20210625032409.15715-1-kevin1.wang@amd.com> <878b44fb-a768-6bc2-2873-b7a75fc5ee1e@gmail.com> From: =?UTF-8?Q?Christian_K=c3=b6nig?= Message-ID: Date: Mon, 28 Jun 2021 17:34:37 +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-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: , Cc: "Deucher, Alexander" , "Lazar, Lijo" , "Min, Frank" , "Koenig, Christian" , "Zhang, Hawking" Content-Type: multipart/mixed; boundary="===============1819303360==" Errors-To: amd-gfx-bounces@lists.freedesktop.org Sender: "amd-gfx" This is a multi-part message in MIME format. --===============1819303360== Content-Type: multipart/alternative; boundary="------------E8F99616BE672B93507D7328" Content-Language: en-US This is a multi-part message in MIME format. --------------E8F99616BE672B93507D7328 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit Hi Kevin, the primary use of amdgpu_device_vram_access() is to gate direct kernel access to VRAM using the MM_INDEX/MM_DATA registers or aperture. It should by design never deal with data sizes != 4 bytes. The amdgpu_ttm_access_memory() is for debug access to the backing store of BOs and also needs to handle per byte access. What we can do is to also use amdgpu_device_vram_access() in amdgpu_device_vram_access() for the masked access, which would also solve a hot plug bug as far as I can see here. But moving the byte access logic into amdgpu_device_vram_access() makes no sense at all. Regards, Christian. Am 28.06.21 um 16:43 schrieb Wang, Kevin(Yang): > > [AMD Official Use Only] > > > Hi Chris, > > amdgpu_ttm_access_memory(struct ttm_buffer_object *bo, unsigned long > offset, void *buf, int len,  int write) > > the above function will be called from kernel side (likes > 'get_user_pages' code path),  and the function should accept > non-aligned addresses. > without this patch, this function will be using MM_INDEX/DATA to > handle aligned address whether in visible memory or not. > for this kind of case , I think the driver should check whether VRAM > aperture works first, then using MM_INDEX/DATA as a backup option. > > for this patch, I only extend amdgpu_device_vram_access() function to > support un-aligned case, and the new function is fully compatible with > the old code logic. > I can't understand why you give a NAK about this patch, I think it's a > good optimization, how do you think? > thanks. > > Best Regards, > Kevin > ------------------------------------------------------------------------ > *From:* Christian König > *Sent:* Monday, June 28, 2021 10:10 PM > *To:* Wang, Kevin(Yang) ; > amd-gfx@lists.freedesktop.org > *Cc:* Deucher, Alexander ; Koenig, > Christian > *Subject:* Re: [PATCH] drm/amdgpu: add non-aligned address supported > in amdgpu_device_vram_access() > Am 25.06.21 um 05:24 schrieb Kevin Wang: > > 1. add non-aligned address support in amdgpu_device_vram_access() > > 2. reduce duplicate code in amdgpu_ttm_access_memory() > > > > because the MM_INDEX{HI}/DATA are protected register, it is not working > > in sriov environment when mmio protect feature is enabled (in some > asics), > > and the old function amdgpu_ttm_access_memory() will force using > MM_INDEX/DATA > > to handle non-aligned address by default, it will cause the > register(MM_DATA) > > is always return 0. > > > > with the patch, the memory will be accessed in the aper way first. > > (in visible memory or enable pcie large-bar feature), then using > > MM_INDEX{HI}/MM_DATA to access rest vram memroy. > > Well NAK to the whole approach. > > The amdgpu_device_vram_access() are intentionally *NOT* using the VRAM > aperture nor providing byte wise access. > > And yes that this doesn't work under SRIOV is completely intentional as > well. > > What we could do is to use the aperture in amdgpu_ttm_access_memory() > for unaligned access if that is indeed a problem. > > Regards, > Christian. > > > > > > Signed-off-by: Kevin Wang > > --- > >   drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  2 +- > >   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 69 ++++++++++++++++------ > >   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 42 ++----------- > >   3 files changed, 58 insertions(+), 55 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > > index d14b4968a026..8095d9a9c857 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > > @@ -1103,7 +1103,7 @@ void amdgpu_device_fini(struct amdgpu_device > *adev); > >   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); > > +                            void *buf, size_t size, bool write); > >   uint32_t amdgpu_device_rreg(struct amdgpu_device *adev, > >                            uint32_t reg, uint32_t acc_flags); > >   void amdgpu_device_wreg(struct amdgpu_device *adev, > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > index e6702d136a6d..2047e3c2b365 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > @@ -280,6 +280,54 @@ bool amdgpu_device_supports_smart_shift(struct > drm_device *dev) > > amdgpu_acpi_is_power_shift_control_supported()); > >   } > > > > +static inline void > amdgpu_device_vram_mmio_align_access_locked(struct amdgpu_device > *adev, loff_t pos, > > + uint32_t *value, bool write) > > +{ > > +     BUG_ON(!IS_ALIGNED(pos, 4)); > > + > > +     WREG32_NO_KIQ(mmMM_INDEX, ((uint32_t)pos) | 0x80000000); > > +     WREG32_NO_KIQ(mmMM_INDEX_HI, pos >> 31); > > +     if (write) > > +             WREG32_NO_KIQ(mmMM_DATA, *value); > > +     else > > +             *value = RREG32_NO_KIQ(mmMM_DATA); > > +} > > + > > +static void amdgpu_device_vram_mmio_access_locked(struct > amdgpu_device *adev, loff_t pos, > > + void *buf, size_t size, bool write) > > +{ > > +     while (size) { > > +             uint64_t aligned_pos = ALIGN_DOWN(pos, 4); > > +             uint64_t bytes = 4 - (pos & 0x3); > > +             uint32_t shift = (pos & 0x3) * 8; > > +             uint32_t mask = 0xffffffff << shift; > > +             uint32_t value = 0; > > + > > +             if (size < bytes) { > > +                     mask &= 0xffffffff >> (bytes - size) * 8; > > +                     bytes = size; > > +             } > > + > > +             if (mask != 0xffffffff) { > > + amdgpu_device_vram_mmio_align_access_locked(adev, aligned_pos, > &value, false); > > +                     if (write) { > > +                             value &= ~mask; > > +                             value |= (*(uint32_t *)buf << shift) & > mask; > > + amdgpu_device_vram_mmio_align_access_locked(adev, aligned_pos, > &value, true); > > +                     } else { > > +                             value = (value & mask) >> shift; > > +                             memcpy(buf, &value, bytes); > > +                     } > > +             } else { > > + amdgpu_device_vram_mmio_align_access_locked(adev, aligned_pos, > buf, write); > > +             } > > + > > +             pos += bytes; > > +             buf += bytes; > > +             size -= bytes; > > +     } > > +} > > + > >   /* > >    * VRAM access helper functions > >    */ > > @@ -294,13 +342,11 @@ bool amdgpu_device_supports_smart_shift(struct > drm_device *dev) > >    * @write: true - write to vram, otherwise - read from vram > >    */ > >   void amdgpu_device_vram_access(struct amdgpu_device *adev, loff_t pos, > > -                            uint32_t *buf, size_t size, bool write) > > +                            void *buf, size_t size, bool write) > >   { > >        unsigned long flags; > > -     uint32_t hi = ~0; > >        uint64_t last; > > > > - > >   #ifdef CONFIG_64BIT > >        last = min(pos + size, adev->gmc.visible_vram_size); > >        if (last > pos) { > > @@ -321,25 +367,12 @@ void amdgpu_device_vram_access(struct > amdgpu_device *adev, loff_t pos, > >                        return; > > > >                pos += count; > > -             buf += count / 4; > > +             buf += count; > >                size -= count; > >        } > >   #endif > > - > > spin_lock_irqsave(&adev->mmio_idx_lock, flags); > > -     for (last = pos + size; pos < last; pos += 4) { > > -             uint32_t tmp = pos >> 31; > > - > > -             WREG32_NO_KIQ(mmMM_INDEX, ((uint32_t)pos) | 0x80000000); > > -             if (tmp != hi) { > > -                     WREG32_NO_KIQ(mmMM_INDEX_HI, tmp); > > -                     hi = tmp; > > -             } > > -             if (write) > > -                     WREG32_NO_KIQ(mmMM_DATA, *buf++); > > -             else > > -                     *buf++ = RREG32_NO_KIQ(mmMM_DATA); > > -     } > > +     amdgpu_device_vram_mmio_access_locked(adev, pos, buf, size, > write); > > spin_unlock_irqrestore(&adev->mmio_idx_lock, flags); > >   } > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > > index 6297363ab740..cf5b8bdc2dd3 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > > @@ -1430,8 +1430,6 @@ static int amdgpu_ttm_access_memory(struct > ttm_buffer_object *bo, > >        struct amdgpu_bo *abo = ttm_to_amdgpu_bo(bo); > >        struct amdgpu_device *adev = amdgpu_ttm_adev(abo->tbo.bdev); > >        struct amdgpu_res_cursor cursor; > > -     unsigned long flags; > > -     uint32_t value = 0; > >        int ret = 0; > > > >        if (bo->mem.mem_type != TTM_PL_VRAM) > > @@ -1439,41 +1437,13 @@ static int amdgpu_ttm_access_memory(struct > ttm_buffer_object *bo, > > > >        amdgpu_res_first(&bo->mem, offset, len, &cursor); > >        while (cursor.remaining) { > > -             uint64_t aligned_pos = cursor.start & ~(uint64_t)3; > > -             uint64_t bytes = 4 - (cursor.start & 3); > > -             uint32_t shift = (cursor.start & 3) * 8; > > -             uint32_t mask = 0xffffffff << shift; > > - > > -             if (cursor.size < bytes) { > > -                     mask &= 0xffffffff >> (bytes - cursor.size) * 8; > > -                     bytes = cursor.size; > > -             } > > +             amdgpu_device_vram_access(adev, cursor.start, > > +                                       buf, cursor.size, > > +                                       write); > > > > -             if (mask != 0xffffffff) { > > - spin_lock_irqsave(&adev->mmio_idx_lock, flags); > > -                     WREG32_NO_KIQ(mmMM_INDEX, > ((uint32_t)aligned_pos) | 0x80000000); > > -                     WREG32_NO_KIQ(mmMM_INDEX_HI, aligned_pos >> 31); > > -                     value = RREG32_NO_KIQ(mmMM_DATA); > > -                     if (write) { > > -                             value &= ~mask; > > -                             value |= (*(uint32_t *)buf << shift) & > mask; > > - WREG32_NO_KIQ(mmMM_DATA, value); > > -                     } > > - spin_unlock_irqrestore(&adev->mmio_idx_lock, flags); > > -                     if (!write) { > > -                             value = (value & mask) >> shift; > > -                             memcpy(buf, &value, bytes); > > -                     } > > -             } else { > > -                     bytes = cursor.size & ~0x3ULL; > > - amdgpu_device_vram_access(adev, cursor.start, > > - (uint32_t *)buf, bytes, > > - write); > > -             } > > - > > -             ret += bytes; > > -             buf = (uint8_t *)buf + bytes; > > -             amdgpu_res_next(&cursor, bytes); > > +             ret += cursor.size; > > +             buf += cursor.size; > > +             amdgpu_res_next(&cursor, cursor.size); > >        } > > > >        return ret; > --------------E8F99616BE672B93507D7328 Content-Type: text/html; charset=windows-1252 Content-Transfer-Encoding: 8bit Hi Kevin,

the primary use of amdgpu_device_vram_access() is to gate direct kernel access to VRAM using the MM_INDEX/MM_DATA registers or aperture. It should by design never deal with data sizes != 4 bytes.

The amdgpu_ttm_access_memory() is for debug access to the backing store of BOs and also needs to handle per byte access.

What we can do is to also use amdgpu_device_vram_access() in amdgpu_device_vram_access() for the masked access, which would also solve a hot plug bug as far as I can see here.

But moving the byte access logic into amdgpu_device_vram_access() makes no sense at all.

Regards,
Christian.

Am 28.06.21 um 16:43 schrieb Wang, Kevin(Yang):

[AMD Official Use Only]


Hi Chris,

amdgpu_ttm_access_memory(struct ttm_buffer_object *bo, unsigned long offset, void *buf, int len,  int write)  

the above function will be called from kernel side (likes 'get_user_pages' code path),  and the function should accept non-aligned addresses.
without this patch, this function will be using MM_INDEX/DATA to handle aligned address whether in visible memory or not.
for this kind of case , I think the driver should check whether VRAM aperture works first, then using MM_INDEX/DATA as a backup option.

for this patch, I only extend amdgpu_device_vram_access() function to support un-aligned case, and the new function is fully compatible with the old code logic.
I can't understand why you give a NAK about this patch, I think it's a good optimization, how do you think?
thanks.

Best Regards,
Kevin
From: Christian König <ckoenig.leichtzumerken@gmail.com>
Sent: Monday, June 28, 2021 10:10 PM
To: Wang, Kevin(Yang) <Kevin1.Wang@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>
Subject: Re: [PATCH] drm/amdgpu: add non-aligned address supported in amdgpu_device_vram_access()
 
Am 25.06.21 um 05:24 schrieb Kevin Wang:
> 1. add non-aligned address support in amdgpu_device_vram_access()
> 2. reduce duplicate code in amdgpu_ttm_access_memory()
>
> because the MM_INDEX{HI}/DATA are protected register, it is not working
> in sriov environment when mmio protect feature is enabled (in some asics),
> and the old function amdgpu_ttm_access_memory() will force using MM_INDEX/DATA
> to handle non-aligned address by default, it will cause the register(MM_DATA)
> is always return 0.
>
> with the patch, the memory will be accessed in the aper way first.
> (in visible memory or enable pcie large-bar feature), then using
> MM_INDEX{HI}/MM_DATA to access rest vram memroy.

Well NAK to the whole approach.

The amdgpu_device_vram_access() are intentionally *NOT* using the VRAM
aperture nor providing byte wise access.

And yes that this doesn't work under SRIOV is completely intentional as
well.

What we could do is to use the aperture in amdgpu_ttm_access_memory()
for unaligned access if that is indeed a problem.

Regards,
Christian.


>
> Signed-off-by: Kevin Wang <kevin1.wang@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 69 ++++++++++++++++------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 42 ++-----------
>   3 files changed, 58 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index d14b4968a026..8095d9a9c857 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1103,7 +1103,7 @@ void amdgpu_device_fini(struct amdgpu_device *adev);
>   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);
> +                            void *buf, size_t size, bool write);
>   uint32_t amdgpu_device_rreg(struct amdgpu_device *adev,
>                            uint32_t reg, uint32_t acc_flags);
>   void amdgpu_device_wreg(struct amdgpu_device *adev,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index e6702d136a6d..2047e3c2b365 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -280,6 +280,54 @@ bool amdgpu_device_supports_smart_shift(struct drm_device *dev)
>                amdgpu_acpi_is_power_shift_control_supported());
>   }
>  
> +static inline void amdgpu_device_vram_mmio_align_access_locked(struct amdgpu_device *adev, loff_t pos,
> +                                                            uint32_t *value, bool write)
> +{
> +     BUG_ON(!IS_ALIGNED(pos, 4));
> +
> +     WREG32_NO_KIQ(mmMM_INDEX, ((uint32_t)pos) | 0x80000000);
> +     WREG32_NO_KIQ(mmMM_INDEX_HI, pos >> 31);
> +     if (write)
> +             WREG32_NO_KIQ(mmMM_DATA, *value);
> +     else
> +             *value = RREG32_NO_KIQ(mmMM_DATA);
> +}
> +
> +static void amdgpu_device_vram_mmio_access_locked(struct amdgpu_device *adev, loff_t pos,
> +                                               void *buf, size_t size, bool write)
> +{
> +     while (size) {
> +             uint64_t aligned_pos = ALIGN_DOWN(pos, 4);
> +             uint64_t bytes = 4 - (pos & 0x3);
> +             uint32_t shift = (pos & 0x3) * 8;
> +             uint32_t mask = 0xffffffff << shift;
> +             uint32_t value = 0;
> +
> +             if (size < bytes) {
> +                     mask &= 0xffffffff >> (bytes - size) * 8;
> +                     bytes = size;
> +             }
> +
> +             if (mask != 0xffffffff) {
> +                     amdgpu_device_vram_mmio_align_access_locked(adev, aligned_pos, &value, false);
> +                     if (write) {
> +                             value &= ~mask;
> +                             value |= (*(uint32_t *)buf << shift) & mask;
> +                             amdgpu_device_vram_mmio_align_access_locked(adev, aligned_pos, &value, true);
> +                     } else {
> +                             value = (value & mask) >> shift;
> +                             memcpy(buf, &value, bytes);
> +                     }
> +             } else {
> +                     amdgpu_device_vram_mmio_align_access_locked(adev, aligned_pos, buf, write);
> +             }
> +
> +             pos += bytes;
> +             buf += bytes;
> +             size -= bytes;
> +     }
> +}
> +
>   /*
>    * VRAM access helper functions
>    */
> @@ -294,13 +342,11 @@ bool amdgpu_device_supports_smart_shift(struct drm_device *dev)
>    * @write: true - write to vram, otherwise - read from vram
>    */
>   void amdgpu_device_vram_access(struct amdgpu_device *adev, loff_t pos,
> -                            uint32_t *buf, size_t size, bool write)
> +                            void *buf, size_t size, bool write)
>   {
>        unsigned long flags;
> -     uint32_t hi = ~0;
>        uint64_t last;
>  
> -
>   #ifdef CONFIG_64BIT
>        last = min(pos + size, adev->gmc.visible_vram_size);
>        if (last > pos) {
> @@ -321,25 +367,12 @@ void amdgpu_device_vram_access(struct amdgpu_device *adev, loff_t pos,
>                        return;
>  
>                pos += count;
> -             buf += count / 4;
> +             buf += count;
>                size -= count;
>        }
>   #endif
> -
>        spin_lock_irqsave(&adev->mmio_idx_lock, flags);
> -     for (last = pos + size; pos < last; pos += 4) {
> -             uint32_t tmp = pos >> 31;
> -
> -             WREG32_NO_KIQ(mmMM_INDEX, ((uint32_t)pos) | 0x80000000);
> -             if (tmp != hi) {
> -                     WREG32_NO_KIQ(mmMM_INDEX_HI, tmp);
> -                     hi = tmp;
> -             }
> -             if (write)
> -                     WREG32_NO_KIQ(mmMM_DATA, *buf++);
> -             else
> -                     *buf++ = RREG32_NO_KIQ(mmMM_DATA);
> -     }
> +     amdgpu_device_vram_mmio_access_locked(adev, pos, buf, size, write);
>        spin_unlock_irqrestore(&adev->mmio_idx_lock, flags);
>   }
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 6297363ab740..cf5b8bdc2dd3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1430,8 +1430,6 @@ static int amdgpu_ttm_access_memory(struct ttm_buffer_object *bo,
>        struct amdgpu_bo *abo = ttm_to_amdgpu_bo(bo);
>        struct amdgpu_device *adev = amdgpu_ttm_adev(abo->tbo.bdev);
>        struct amdgpu_res_cursor cursor;
> -     unsigned long flags;
> -     uint32_t value = 0;
>        int ret = 0;
>  
>        if (bo->mem.mem_type != TTM_PL_VRAM)
> @@ -1439,41 +1437,13 @@ static int amdgpu_ttm_access_memory(struct ttm_buffer_object *bo,
>  
>        amdgpu_res_first(&bo->mem, offset, len, &cursor);
>        while (cursor.remaining) {
> -             uint64_t aligned_pos = cursor.start & ~(uint64_t)3;
> -             uint64_t bytes = 4 - (cursor.start & 3);
> -             uint32_t shift = (cursor.start & 3) * 8;
> -             uint32_t mask = 0xffffffff << shift;
> -
> -             if (cursor.size < bytes) {
> -                     mask &= 0xffffffff >> (bytes - cursor.size) * 8;
> -                     bytes = cursor.size;
> -             }
> +             amdgpu_device_vram_access(adev, cursor.start,
> +                                       buf, cursor.size,
> +                                       write);
>  
> -             if (mask != 0xffffffff) {
> -                     spin_lock_irqsave(&adev->mmio_idx_lock, flags);
> -                     WREG32_NO_KIQ(mmMM_INDEX, ((uint32_t)aligned_pos) | 0x80000000);
> -                     WREG32_NO_KIQ(mmMM_INDEX_HI, aligned_pos >> 31);
> -                     value = RREG32_NO_KIQ(mmMM_DATA);
> -                     if (write) {
> -                             value &= ~mask;
> -                             value |= (*(uint32_t *)buf << shift) & mask;
> -                             WREG32_NO_KIQ(mmMM_DATA, value);
> -                     }
> -                     spin_unlock_irqrestore(&adev->mmio_idx_lock, flags);
> -                     if (!write) {
> -                             value = (value & mask) >> shift;
> -                             memcpy(buf, &value, bytes);
> -                     }
> -             } else {
> -                     bytes = cursor.size & ~0x3ULL;
> -                     amdgpu_device_vram_access(adev, cursor.start,
> -                                               (uint32_t *)buf, bytes,
> -                                               write);
> -             }
> -
> -             ret += bytes;
> -             buf = (uint8_t *)buf + bytes;
> -             amdgpu_res_next(&cursor, bytes);
> +             ret += cursor.size;
> +             buf += cursor.size;
> +             amdgpu_res_next(&cursor, cursor.size);
>        }
>  
>        return ret;


--------------E8F99616BE672B93507D7328-- --===============1819303360== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx --===============1819303360==--